Merged
Conversation
7 tasks
|
Size Change: +232 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
Contributor
aaronrobertshaw
left a comment
There was a problem hiding this comment.
Thanks @jasmussen for the quick fix.
I switched back to the TT1 theme on trunk and immediately saw the regression. A bad miss on my part.
After checking out this PR, the issue with the text decoration styling not being inherited is back. I left a comment below suggesting we tweak that rule to be display: inline. From my testing that covers both bases and will buy us time as well in case there are delays in getting #36104 across the line.
(#36014 will still be needed to prevent the placeholder taking on the decoration as well)
| Inline Block | Inline |
|---|---|
 |
 |
Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com>
Contributor
Author
|
Excellent catch and thank you for the review. Also, I approved your PR, so we're in this together — easy to fix though, so all is well. I committed and commented your fix, because it does indeed appear to work: |
aaronrobertshaw
approved these changes
Nov 1, 2021
Contributor
aaronrobertshaw
left a comment
There was a problem hiding this comment.
LGTM 👍
- Dropdown indicator does not wrap
- Text decoration styles are inherited
Contributor
Author
|
The highest of fives 🙌 |
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
2 participants
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
Followup to #35859, apparently the rule was needed after all. Turns out the inline-block rule affected the positioning of the dropdown arrow:
By restoring the rule, things look right again:
How has this been tested?
Please test a navigation with dropdown menus and verify the icon indicator looks right in both editor and frontend.
Checklist:
*.native.jsfiles for terms that need renaming or removal).