Update the template title in the details panel #49487
Merged
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
jameskoster
added
Needs Design Feedback
|
Size Change: +1.57 kB (0%) Total Size: 1.34 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in b49e78f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4595726340
|
|
Definitely a step forward! Happy to land this as is. Another option is to just top-align the box, so that the icons always stay put. I think I might lean slightly more towards that, mainly because on these detail pages, there's room to be verbose:
What do you think? |
Co-Authored-By: Joen A. <1204802+jasmussen@users.noreply.github.com> Co-Authored-By: James Koster <846565+jameskoster@users.noreply.github.com>
|
I don't mind it:
The optics look slightly off, but I think it's due to the unorthodox positioning of the Edit icon. I'll look into that in a separate PR. |
jasmussen
reviewed
Mar 31, 2023
| @@ -37,10 +37,6 @@ | |||
| } | |||
|
|
|||
| .edit-site-sidebar-navigation-screen__title { | |||
| font-size: calc(1.56 * 13px); | |||
There was a problem hiding this comment.
This was a bit of a weird font size, a roundabout way to arrive at 20px, however I do miss those 20px font size. I thought that was more substantial as a detail page heading.
There was a problem hiding this comment.
Heh, I know it's subjective but the 20px feels kind of oversized to me, particularly with the longer titles.
I like 16 because it aligns nicely with the icons:
It's also what we've been circling around with mockups like:
This is only a light pushback though, I don't mind working on that separately :)
Edit: It also means using non-grid-unit value to get the alignment right (6px rather than 8) 🙈
There was a problem hiding this comment.
6px is a grid unit! It's $grid-unit-15 * 0.5 ;)
It's not a change I'd fight tooth and nail against. But mainly I think it's important we have some contrast between the various headings, just so we don't end up with a bunch of font sizes that are close but not quite contrasty enough. At the moment we have 11px labels (allcaps), 12px help text, 13px body size for the small stuff. Then we have a few sizes above here that don't seem entirely systematized, something around 14px for modal subheadings, and 19ish for modal headers. It's a bit messy and could use a system in our library. 16 may be a part of that, but then I'd roll it out more broadly. Just visually it doesn't seem far enough from 13px.
If you want the 16px change in this PR, I'm happy to make @richtabor or @SaxonF a tie-breaker :D
There was a problem hiding this comment.
6px is a grid unit! It's $grid-unit-15 * 0.5 ;)
😆
Oh absolutely, we should define a scale, ideally built into the Text / Heading components. We have this in the Design Library, but like you said only the only ones close to being formalised in any way are Label, Helper text, and Body. We should revisit that now we're beginning to explore management screens in some more detail.
No need for a tie-breaker haha, I reverted.
There was a problem hiding this comment.
I don't mind the current state's 20px size. I share the same sentiment as @jasmussen — 16px may be too small.
Co-Authored-By: Joen A. <1204802+jasmussen@users.noreply.github.com> Co-Authored-By: James Koster <846565+jameskoster@users.noreply.github.com>
jasmussen
approved these changes
Mar 31, 2023
bph
added
[Type] Bug
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.
What?
This PR does a couple of things:
Why?
With custom templates and so on it's increasingly common to see longer titles which wrap and look a bit awkward:
How?
The changes involve some CSS, and swapping an
h2withHeadingcomponent.Testing Instructions
template.title.mp4