Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Social: Consolidate social initial state #38606
Social: Consolidate social initial state #38606
Changes from all commits
18026f2
a4e0b93
2c677a3
bb90514
59dc46c
590d56f
5b0ac8a
a8f41a8
713c909
ac4b37b
d32dc2d
16a9564
1ba5b7d
f927d65
436c436
7acde2b
d4fd7a9
14a1cd6
745918a
066270e
7e2adfa
aadfeb3
948e8ff
d45437a
29daeaf
7a0834f
acaef37
d5abc90
d0fc2d6
bdf049a
79ec614
78271ff
c625bfe
3813e96
2276331
76022bf
5fb4553
df778a8
659bea0
7d6ccf4
84d5a5a
0cd03bc
ea91938
8cc9b45
5ac15db
528fc56
d7aa3d7
13cdfec
6ab848f
cbb4bda
e49d6e1
8803b1d
d089d6c
8bf1756
ead4d9d
606536d
f834580
e504f32
9b28581
5d18f6e
45327ce
5406f7c
769d2c7
12e563d
0ac1e95
de290e9
f09fecc
0739b4e
fe7faa7
3c4201c
2190351
98d6408
9bec185
b9f3186
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we have to add the new feature flag for the modal as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we need to migrate all our initial state to this new consolidated one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused here what the class name "Script data" means. Could we have a more descriptive doc if this describes the class the best?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually the package we created in #38430 was initially named as
initial-state
and the class was also named asJetpack_Initial_State
but after some discussion with Garage team, we reached the conclusion to use the wordscript data
which is what the initial state actually is. Also, WordPress uses the similar nomenclature for such data -wp_add_inline_script
has calls the second argument as$data
and likewise the other such functions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have this in publicize-base with my previous changes, would that be deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we will eventually move to this new consolidated logic.