Update FormTokenField styles after @wordpress/components update #8051
Conversation
Aljullu
added
focus: components
woocommercebot
requested review from
a team and
sunyatasattva
and removed request for
a team
|
The release ZIP for this PR is accessible via: Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
Detailsassets/js/base/components/button/index.tsxassets/js/base/components/cart-checkout/address-form/test/index.js assets/js/base/context/event-emit/test/emitters.js assets/js/blocks-registry/block-components/test/index.js assets/js/blocks-registry/payment-methods/test/payment-method-config-helper.ts assets/js/blocks-registry/payment-methods/test/registry.ts assets/js/blocks/attribute-filter/test/block.tsx assets/js/blocks/cart/test/block.js assets/js/blocks/cart/test/slots.js assets/js/blocks/checkout/inner-blocks/checkout-order-summary-block/test/block.js assets/js/blocks/checkout/inner-blocks/checkout-terms-block/test/edit.js assets/js/blocks/checkout/inner-blocks/checkout-terms-block/test/frontend.js assets/js/blocks/mini-cart/test/block.js assets/js/blocks/rating-filter/test/block.tsx assets/js/blocks/stock-filter/test/block.tsx packages/checkout/components/text-input/test/validated-text-input.tsx |
|
Size Change: +2 B (0%) Total Size: 836 kB
ℹ️ View Unchanged
|
sunyatasattva
left a comment
sunyatasattva
left a comment
There was a problem hiding this comment.
Tested and it works correctly! Added a small question to the code, but otherwise all seems good.
I'd probably like to see us using less hard-coded numbers there, but that's a much bigger topic. 😬
| .components-form-token-field__token-text, | ||
| input[type="text"].components-form-token-field__input { | ||
| min-height: 30px; | ||
| min-height: calc(31.5px - 0.5em); |
There was a problem hiding this comment.
Out of curiosity, could you explain what problem is this solving? I tried removing this and breaking it in various ways, and I didn't manage. What is the min-height needed for? Also, if that's needed, wouldn't this kind of pixel perfect approach be somewhat fragile? Since em depend on the font-size it seems that you can get into a very small min height on larger font sizes.
But, again, as I don't really get why this is here, perhaps I'm getting it all wrong.
There was a problem hiding this comment.
Good catch, I think that was a left-over from a previous approach to fix it. I removed it in 37654f6.
|
Thanks for the review, @sunyatasattva!
💯 completely agree. I will try to take a look at this as a follow-up. We should also unify the duplicated styles across blocks (related to #7875). |
Aljullu
force-pushed
the
fix/update-form-token-field-styles-after-wordpress/components-update
branch
from
65fd96e to
37654f6
Compare
|
@kmanijak do you want me to merge this branch into yours? |
|
Thank you for working on this. I was testing it in my environment and I got the following: It seems that depending on the number of characters, the selector creates an additional space that makes it becomes bigger. Can you reproduce it? This is my environment:
|
|
I can reproduce, but I am not sure this is an error. This is due to the fact that in that extra space you see at the bottom, the user can focus the cursor and type. It could be argued that this is not necessary as there is still space on the top row, but the space is very small and is competing with the dropdown caret, so I'd say in this case it's ok for accessibility and clarity reasons. But that's just my opinion. @Aljullu what do you think? On the other hand, I noticed something I'd like to be different, that the caret has no |
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
Yes, I agree. When the existing chip is too wide, the input breaks into another line. I think that's fine as you mention for a11y reasons and to make it clear that new values can be added.
It's not clear to me what do you mean by "caret" in this context? |
The little “arrow down” on the right side that usually signifies the part of a dropdown with which you can interact and, often, indicates the expanded/collapsed state of the suggestions. |
github-actions
bot
removed
the
status: stale
thealexandrelara
left a comment
thealexandrelara
left a comment
There was a problem hiding this comment.
Great, since that is not considered an issue, I'm also approving the PR. Thank you Lucio and Albert for your insights
|
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
github-actions
bot
added
the
status: stale
nielslange
deleted the
fix/update-form-token-field-styles-after-wordpress/components-update
branch
This PR updates the styles of the
FormTokenFieldcomponent to fix some regressions caused by@wordpress/componentsupdate. cc @kmanijakScreenshots
Testing
User Facing Testing
WooCommerce Visibility