Add select--stroke modifier and change to default borderless select

[adrianababakanian]

Closes #982

This PR adds the select--border-0 modifier class to support borderless select elements out-of-the-box. It removes the select's default box-shadow and left padding. select--stroke modifier and updates select elements to be unstroked by default.

@samanpwbb for review -- I'm curious if you think the padding right of the select-arrow should also be reduced or removed, in which case there would be some additional work to be done here.

[samanpwbb]

@adrianababakanian this looks like a solid start!

I'm curious if you think the padding right of the select-arrow should also be reduced or removed

Looking to the figma files for guidance, I think we do want a small amount of right padding yes!

Before I approve or reject this, I have a couple questions:

1. Did you consider the trade-offs of making the stroke a modifier and the unstroked style the new base style? That might be more in keeping with Assembly's additive styling model, but at the cost of a change that might be considered "breaking" (although that's up for debate).

2. Did you consider styling disabled/active states differently for unstroked selects? I wonder, for example, if we should only apply background-color changes to stroked selects.

[adrianababakanian]

Thanks for the feedback @samanpwbb !

Looking to the figma files for guidance, I think we do want a small amount of right padding yes!-

👍

Did you consider the trade-offs of making the stroke a modifier and the unstroked style the new base style?

I didn't consider this, though I do see how making the unstroked style the default would be more in keeping with the additive styling model that you mentioned. Should we move forward with switching the modifier to something like select--stroke, similar to btn--stroke?

Did you consider styling disabled/active states differently for unstroked selects?

Great callout. I do think the active state makes sense to keep the same as the stroked select -- i.e., the arrow color becomes darker and font color remains the same.

But, we might want to handle the disabled state differently, as it doesn't look great with the current state of this PR given the lack of left padding:

I'm not seeing anything explicitly in the style guide for how to handle this case -- do you have any thoughts?

[samanpwbb]

Should we move forward with switching the modifier to something like select--stroke, similar to btn--stroke?

I am on board – @katydecorah would you be okay with this change? It may require some work to migrate for you.

I'm not seeing anything explicitly in the style guide for how to handle this case -- do you have any thoughts?

Ya, maybe something closer to how we do disabled link? disabled-colored text and disabled-colored triangle, and leave background transparent?

[katydecorah]

would you be okay with this change?

👍 Yes, I think that pattern makes sense

[adrianababakanian]

When modifying select to be borderless by default, I made a few modifications:

• Added a select--stroke modifier to add a border + additional padding to the select element.
• Changed the select--border-${color} modifiers to select--${color}. They now have the effect of coloring just the arrow when used with select, and both the arrow and border when used in conjunction with the select--stroke modifier.

@samanpwbb I wonder if it would make sense for the select--${color} modifiers to also change the font color for both the bordered and borderless versions. Referencing Figma, it looks like one of the bordered selects has a different font + arrow color from its border, whereas the other uses the same color for all three elements. Should the former be the default coloring, and the latter the result if a user explicitly sets a color via a modifier?

[samanpwbb]

I wonder if it would make sense for the select--${color} modifiers to also change the font color for both the bordered and borderless versions.

For unstroked, definitely.

Not sure about stroked - we could go either way.. I know @katydecorah originally modelled these after text inputs, which do not change text color.

Referencing Figma, it looks like one of the bordered selects has a different font + arrow color from its border, whereas the other uses the same color for all three elements.

What you're seeing is the difference between the active and inactive state. Active state should use -dark variants for everything, while inactive state should have a light border, and default colored text and triangle.

Also, looking at your screenshot, I wonder if we should adjust the default select style to use a darker triangle?

[adrianababakanian]

Thanks for the clarifications and continued reviews @samanpwbb ! I updated the default arrow to use the default form color rather than the lighter default form color, so now it's the same as the font color. The active state now also uses -dark variants for everything.

Right now the colored stroked selects use an updated font color as well. I like how it creates consistency with how unstroked selects handle color variants, but at the same time I see the value in keeping them modeled after text inputs (which don't change colors). @katydecorah I'd appreciate any thoughts you might have on this!

[katydecorah]

Right now the colored stroked selects use an updated font color as well. I like how it creates consistency with how unstroked selects handle color variants

@adrianababakanian While I love the way it looks, we should make sure that the font color for text has an accessible contrast ratio. My vote is to optimize for legibility over consistency. I know the Assembly color palette will be updated in v1, so perhaps the font color will soon have improved color contrast that would allow us to successfully update the font color without losing legibility.

[samanpwbb]

@adrianababakanian this is coming together! Noticed a couple minor issues.

Another thing I noticed, that is also present on previous version: we don't have good [data-assembly-focus-control='visible'] styling for selects. Is this something we could fix? Notice how other form types take on a prominent :focus style, but selects do not.

[adrianababakanian]

My vote is to optimize for legibility over consistency.

Really great point @katydecorah ! I think that makes sense. Optimizing for legibility, I see three potential paths forward:

1. Use the default font color for both stroked and unstroked selects regardless of the color modifier. This means that applying color variants to the default selects only affects the arrow color, which does look a little funny.
2. Use the -medium variants for each color from the new style guide as the text + arrow colors when color modifiers are applied (for both stroked + unstroked elements).
3. Use the -medium variants for each color from the new style guide as the text + arrow colors for just the default select when color modifiers are applied. Also use the -medium color variant for the arrow in the stroked selects, but use the default form color for the text color.

[samanpwbb]

@adrianababakanian this is looking really good. Almost ready to merge. I see two tiny issues:

Default stroked tab state doesn't show double border:

Then, after seeing this work and thinking through your notes about differences between stroked/unstroked selects, and thinking through how we use selects throughout our systems now, here's my recommendation:

• Model unstroked select styling after links: leave default and active coloring as you have it now, and add hover states. This also more closely models how Studio now styles unstroked selects
• Model stroked select styling after text inputs: use default text coloring always, and only apply color variants to border and triangle.

[adrianababakanian]

Thanks @samanpwbb ! I think I might have pushed up bbf9a30 (#992), which should resolve

Default stroked tab state doesn't show double border

, while you had already started reviewing -- let me know if the most recent commit fixes that ☝️ issue! I noticed that in hindsight too.

I'll add those recommendations for unstroked/stroked select styling too.

[adrianababakanian]

@samanpwbb this should be ready for your final review!

Default stroked tab state doesn't show double border

Per #992 (comment), should be good on this point.

Model unstroked select styling after links: leave default and active coloring as you have it now, and add hover states. This also more closely models how Studio now styles unstroked selects

I made these udpates, but kept the default box-shadow on focus. Links don't use box-shadows -- let me know if you think we should keep or remove them for unstroked selects.

Model stroked select styling after text inputs: use default text coloring always, and only apply color variants to border and triangle.

✅

this documentation should clarify that the select--stroke modifier changes how the color modifier gets interpreted.

This clarification is discussed in the docs for both the default select group and the select--stroke modifier. Open to feedback on restructuring docs to make this clearer!

[samanpwbb]

Noticed one small detail, once that's taken care of this should be good to go.

[samanpwbb]

This is great!

I had one other optional thought when playing around with this: should we remove the default border-radius on non-stroked selects? Just to keep the base style a bit more minimal? We'd then apply it to the stroked variant only. I don't think this change is necessary but if you agree with me, might be worth sneaking in.

[adrianababakanian]

Thanks @samanpwbb !

should we remove the default border-radius on non-stroked selects?

Hm, my initial thought was yes, but since we're applying a box-shadow to unstroked selects on focus, I think it might be nice to keep the default border-radius. That way when a default select is focused, the overall effect would be more consistent with the style guide.