Global styling: Inputs styling not applied consistently

[sofiamatulis]

Why are these changes introduced?

Fixes #993 .

What approach did you take?

Added missing styles to country/language selector, quantity, cart notification and share block

Other considerations

Because we removed the corner radius on the bottom right from the cart notification, it can sometimes look like this: https://screenshot.click/05-50-wqgpv-vn6wh.png

Demo links

• Editor

Checklist

• Followed theme code principles
• Linted with Theme Check
• Tested on mobile
• Tested on multiple browsers
• Tested for accessibility

[melissaperreault]

Thank you Sofia! 🎉

Some small observations:

• Let's check with @kmeleta , I recall we had a fix to space out siblings when border thickness is increased. Might have to do with this merged PR Refactor button styles #1064. Might need to do some more investigation for spacing out labels for border thickness. I know we intend to have a setting to space out siblings for improving the shadow offset sometimes, but unsure if that could become a systematic change. There are some strong legibility issues otherwise. cc. @nicklepine
• Share block: Let's pair, I am not sure what would be the ideal solution to simplify the focus behaviour here.
• Quantity input: As we are scaling the border thickness, we are losing the input number and the delete icon is no longer centered vertically with the component.

[sofiamatulis]

• Share block: Let's pair, I am not sure what would be the ideal solution to simplify the focus behaviour here.

@melissaperreault The reason that it's thicker there is to ensure that the focus is visible when the borders are thicker. You will see in the calculation below, the border is being calculated by adding 2px to it to ensure the user can see it when tabbing even when the border is thicker

box-shadow: 0 0 0 calc(var(--inputs-border-width) + 2px) rgba(var(--color-foreground), var(--inputs-border-opacity));

Edit: Maybe we can do something similar to button?

[kmeleta]

• Let's check with @kmeleta , I recall we had a fix to space out siblings when border thickness is increased.

The change I made in that PR was only applied to buttons. The border overlap happens because we're only using a box-shadow as a border, which has no physical dimension. For button, I ended up using the actual <button> element to occupy the appropriate min-width + padding + border width dimension and rendered the border and shadow on pseudo elements so they could animate independently of each other.

We should be able to apply a similar approach to inputs, but we may well want to handle that as a separate PR.

cc @melissaperreault

[ludoboludo]

Noted a few things but not sure if you're in the middle of changing things or not. There was a changed pushed while I was reviewing.

Here is what I noted:

• focus styles are a bit off atm: on locales and other inputs
• cart note is now looking a bit funky: screenshot
• locales are centered but it looks off due to the caret icon. Maybe we can edit the padding-left value: screenshot
• There can be overlaps on variants when their layout is a dropdown. Screenshot. Tho with the latest changes it seems like it's changing their size and not applying borders properly: screenshot
• The contact form section seem to miss the bottom right corner radius. It doesn't like it needs to be applied there 🤔

[melissaperreault]

The contact form section seem to miss the bottom right corner radius. It doesn't like it needs to be applied there

True it should only apply to Cart note, not Comment component.

[sofiamatulis]

@melissaperreault

Do you mind taking a look at the hover states/focus states and confirming the behaviour when we have thicker borders? 🙏

[ludoboludo]

Noticed a few oddities.

Here are the settings I had while testing: screenshot

[sofiamatulis]

@melissaperreault I have re-added the resizing for the cart note (I can always remove it), since @tyleralsbury mentioned it was a good accessibility practise cc @svinkle .
We can either:

1. Remove it for now and create a PR to add resize to all textboxes
2. Keep resize in cart note and add the remaining re-sizes in a follow up PR

https://screenshot.click/10-45-pp58y-idia6.mp4

[kmeleta]

Mostly looking good. Couple notes.

I think the biggest thing I see is inputs are receiving too much padding and/or height (noticed on search and customer accounts)

Expected..

[ludoboludo]

Looking good, added a few comments.

Also on the contact form section, the comment part changes in heigh when I click on it to edit it: video

[melissaperreault]

🎉 Thanks Sofia for all the hard work! I think we can say it is a wrap, let's follow up with polishing task. I'll look around and try to map out a plan for some suggestions that I made for future behavior improvement in general.

Last observation:

• When corner radius is applied, only the search icon focus will display the border inside the focus as oppose to the others. (Video example)

[kmeleta]

One more note 😬

[kmeleta]

Thanks for all the updates Sofia! I think this looks good to me. Let's just make sure all follow ups are accounted for in issues. In the future I'd also like to look at utilizing a shared .settings-inputs class to reduce some redundancy for many of these styles.

[sofiamatulis]

Thanks for all the updates Sofia! I think this looks good to me. Let's just make sure all follow ups are accounted for in issues. In the future I'd also like to look at utilizing a shared .settings-inputs class to reduce some redundancy for many of these styles.

@kmeleta I agree 👍 I thought of the same thing since some of the styles are shared. I will be handling this in a follow up

[ludoboludo]

The only thing I noticed was this: screenshot

The rest looked great.

Thanks for all the adjustments along the way 🎉

[sofiamatulis]

screenshot

Created an issue here :) #1193

Thank you all for the great and detailed reviews 👍