-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[MWPW-156760] Table caret position #3568
Conversation
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## stage #3568 +/- ##
==========================================
- Coverage 96.49% 96.48% -0.01%
==========================================
Files 260 260
Lines 60763 60810 +47
==========================================
+ Hits 58632 58673 +41
- Misses 2131 2137 +6 ☔ View full report in Codecov by Sentry. |
5ffb5bd
to
50bb653
Compare
libs/blocks/table/table.css
Outdated
width: 12px; | ||
height: 17px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where are these specific values coming from? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a chevron icon in the project already and without uploading a new one I tried to make it look as close to the design as possible since the original is a bit too big. If your concern here is the focus box is not even we can get the height to be 12px also, but the icon when tabbing is not being focused but the cell in the table.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the height
declaration is ideal. The icon seems to be 17x10px
, so setting a height larger than the width might not achieve anything extra. We could set the height
to 100%
(also technically incorrect, but more general than 17px
) or remove the height and set an aspect-ratio: 1.7
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small improvement suggestion.
libs/blocks/table/table.css
Outdated
width: 12px; | ||
height: 17px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the height
declaration is ideal. The icon seems to be 17x10px
, so setting a height larger than the width might not achieve anything extra. We could set the height
to 100%
(also technically incorrect, but more general than 17px
) or remove the height and set an aspect-ratio: 1.7
.
Reminder to set the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verified, testing details https://jira.corp.adobe.com/browse/MWPW-156760
This changes the caret for table from "Title +/-" to ">/⌄ Title"
Resolves: MWPW-156760
Test URLs: