Make global styles shape consistent with local styles shape. - #22744
Merged
jorgefilipecosta merged 1 commit intoMay 29, 2020
Merged
Conversation
jorgefilipecosta
requested review from
TimothyBJacobs and
spacedmonkey
as code owners
May 29, 2020 12:39
jorgefilipecosta
force-pushed
the
update/make-global-styles-shape-consitent-with-local-styles-shape
branch
from
May 29, 2020 12:41
b85f545 to
3213429
Compare
|
Size Change: 0 B Total Size: 1.11 MB ℹ️ View Unchanged
|
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
oandregal
reviewed
May 29, 2020
jorgefilipecosta
force-pushed
the
update/make-global-styles-shape-consitent-with-local-styles-shape
branch
2 times, most recently
from
May 29, 2020 15:18
182b137 to
4b505ea
Compare
oandregal
approved these changes
May 29, 2020
oandregal
left a comment
Member
There was a problem hiding this comment.
This is a lovely PR, thanks.
Member
|
@jorgefilipecosta actually, I forgot to mention that we need to update the docs for theme.json. We can land this after that and fixing the linting errors. To make progress, I've already ported the get utility to #22698 I'll deal with the rebase as soon as this lands. |
jorgefilipecosta
force-pushed
the
update/make-global-styles-shape-consitent-with-local-styles-shape
branch
2 times, most recently
from
May 29, 2020 16:35
ac21383 to
a415c69
Compare
jorgefilipecosta
requested review from
ajitbohra,
chrisvanpatten and
mkaz
as code owners
May 29, 2020 16:35
Co-authored-by: Andrés <nosolosw@users.noreply.github.com> (+1 squashed commit) Squashed commits: [3213429] Update: Make global styles shape consistent with local styles shape.
jorgefilipecosta
force-pushed
the
update/make-global-styles-shape-consitent-with-local-styles-shape
branch
from
May 29, 2020 16:35
a415c69 to
b60733d
Compare
jorgefilipecosta
deleted the
update/make-global-styles-shape-consitent-with-local-styles-shape
branch
May 29, 2020 16:53
Member
Author
|
Hi @nosolosw Thank you for the review the PR was merged and the documentation was updated 👍 |
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently, for the shape of the local style we are using something like this:
While for global styles we were using the shape:
We should use the same shape and this PR updates the global styles shape to be similar to what we had for local styles.
There is still one inconsistency for local styles we call "style" and for global styles, we call "styles". Should we update global styles to use "style"?
Besides this update we also perform smaller updates required to have a link color end to end implementation:
- We add a setting __experimentalGlobalStyles containing the full global styles required to show the defaults including user-set global styles.
- We unconditional include the default styles even if the theme does not has theme.json support. I guess even if the theme does not contain theme.json we may want to allow users to change the link color, and even change it globally and for that, we need a default even if for themes that don't have theme.json.
How has this been tested?
I added this after global to /lib/experimental-default-theme.json:
I verified the following styles were output: