Add Letter-spacing and enable this for site title and site tagline #31118
Conversation
This file contains 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
carolinan
added
the
Global Styles
|
Size Change: +298 B (0%) Total Size: 1.86 MB
ℹ️ View Unchanged
|
This was referenced Apr 23, 2021
carolinan
requested review from
ajitbohra,
ellatrix,
oandregal,
spacedmonkey and
TimothyBJacobs
as code owners
carolinan
changed the title
ockham
reviewed
May 3, 2021
ockham
reviewed
May 3, 2021
ockham
reviewed
May 3, 2021
There was a problem hiding this comment.
Thanks @carolinan, this is looking good and working well in my testing! 👍 From a technical point of view, this LGTM.
It might be nice to get a designer to have a quick look at this and maybe help polish it a bit -- I think I saw a convo between @mtias, @jasmussen, and @jameskoster the other day about styling typography related controls. (One point I remember was that we now have some controls that do have a 'Reset' button while others don't, and it seems a bit arbitrary.) For reference, here's an issue with a few mockups: #27331.
|
Thanks for the ping. Yes, we really need a progressive-reveal system as described in #27331, especially important for somewhat exotic properties like this one which nevertheless need to be available:
The prominence in that mockup also weights the letter spacing as "half a column", rather than a full row, but that does seem like it's made easier by the fact that simply unchecking the property in that mockup also serves as a reset. What is the closest we can get to that mockup, in absence of the progressive-reveal system? |
carolinan
requested review from
adamsilverstein,
adamziel,
atimmer,
cameronvoell,
chrisvanpatten and
dmsnell
as code owners
ockham
reviewed
May 10, 2021
packages/block-editor/src/components/letter-spacing-control/index.js
Outdated
Show resolved
Hide resolved
ockham
reviewed
May 10, 2021
There was a problem hiding this comment.
Thanks @carolinan, looking pretty good now! I have one more suggestion 😄
7 tasks
|
|
|
Awaiting #31782 |
|
@carolinan Are you sure you want to wait for #31782? It's very considerate of you, but I don't think it's a strict requirement -- that PR is still only in draft state, so it might take a while before it reaches approval state, and it might become progressively harder to rebase this PR. |
|
I think it would be best to proceed without #31782. That PR will take a long time to be ready - if it ever is. |
This was referenced Oct 4, 2021
|
Hey @carolinan do you remember if it was a conscious decision to not add Letter spacing to the Global Styles typography panel? |
|
It was not, I don't think it even crossed my mind to add it there too :) |
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.
Description
Adds a new letter-spacing control to the typography panel.
Enables the control for site title and site tagline blocks.
Partial for #30918, #30921
How has this been tested?
I have tested three blocks manually in the site editor and block editor:
Site title and tagline with the control.
Site title and paragraph with defaults declared in theme.json.
To enable letter-spacing in theme.json:
Screenshots
Block editor:
Site editor: ❓ Help wanted. I am not sure why the control looks different in the site editor.
Global styles:
Front:
Types of changes
New control
Checklist:
*.native.jsfiles for terms that need renaming or removal).