Restore Post Title visual styles in Code View mode #56582
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
getdave
added
[Type] Regression
|
Size Change: +55 B (0%) Total Size: 1.7 MB
ℹ️ View Unchanged
|
t-hamano
reviewed
Nov 28, 2023
There was a problem hiding this comment.
Thank you for submitting PR! I have two suggestions in terms of design and accessibility.
Regarding the design, the title area seems to have the thicker border than the content area. Also, the focus outline seems thin.
Regarding accessibility, the title element has been changed from h1 to the textarea. I tested it with a screen reader (NVDA) and it reads the placeholder "Add title", but I'm concerned that the semantics of the document have been lost.
| font-size: inherit; | ||
| line-height: inherit; | ||
| // Raw Text Variant | ||
| .edit-post-text-editor__body .editor-post-title.is-raw-text { |
There was a problem hiding this comment.
What does the .is-raw-text class exist for? Even without this class, it seems that we can determine that it is a code editor just by looking at the .edit-post-text-editor__body class.
There was a problem hiding this comment.
Without this class there is no way to determine whether the field is in "rich" or "raw" mode. I prefer not to rely on a cascade of CSS classes in parents that may or not be present in other contexts.
The semantics issue needs a fix asap. |
Excellent spots! Thank you. I've adjusted them a bit. It's tricky because the HTML structure is not the same (depends on #56633) but I think I've now managed to reuse more of the original styles which should avoid things looking inconsistent. |
getdave
force-pushed
the
fix/restore-post-title-visuals
branch
from
71a6e89
to
9eac8b0
Compare
t-hamano
approved these changes
Nov 29, 2023
There was a problem hiding this comment.
LGTM!
- The title in the code editor has the same default border and focus style as the content.
- Visual mode is not affected by this style.
Regarding title semantics, let's consider whether or not we need to address in #56633.
derekblank
pushed a commit
that referenced
this pull request
Dec 7, 2023
* Apply styles to match previous visuals * Adjust CSS in response to feedback
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?
Restores the visual styles to the Post Title field that were lost as part of #54718
Closes #56581
Why?
We should retain the previous visual styles.
How?
These styles were lost due to the usage of a new component. Update the post title styles for the "raw" variant to match the previous design.
Testing Instructions
Reference screenshot
This is how the field looked on
trunkprior to the merge of #54718.Testing Instructions for Keyboard
Screenshots or screencast
Before (current trunk)
After (this PR)
The following screenshot is how the field looks as a result of this PR: