Review of recent changes

[davidtheclark]

I'd love it if @samanpwbb and @mapbox/frontend-platform (and anybody else in @mapbox/frontend who is interested) could take a look at the HEAD section in the changelog and share any feedback you have about the new patterns. It would probably be helpful to look at the changelog then run the docs locally and check things out.

There's a little bit of general clean up, but most of the changes relate to more opinionated styling and more controlled style-modifying props. Most components that are not form components have been updated according to this plan. (Form components can come later.)

Here's a run-down of the patterns:

• A variant prop can be used for keyword-based presets. In Button, the chosen variant ends up setting props that you can then override if you want to set those props directly.
• color props expect an Assembly color name.
• There a bunch of keyword styling props (e.g. "small" | "medium" | "large").
• There are a few boolean styling props (e.g. block, overlapBorder).
• passthroughProps is a convention for passing props directly to a DOM element.
• CopyButton is the only component that includes a className prop for fully overriding the element's classes. If necessary, we could add a similar nuclear-option override to other components; but CopyButton seems like an exception because we have almost no consistency across our usage of it.

See the specific props related to the above patterns

Button:

• variant
• size
• width
• outline
• color
• corners
• block
• passthroughProps

Copiable:

• truncated

CopyButton:

• block
• passthroughProps
• className (full flexibility)

Heading:

• variant

IconText:

• spacing

Modal:

• size
• padding

Popover

• coloring
• padding
• passthroughProps

PopoverTrigger

• block
• passthroughTriggerProps

Tooltip

• coloring
• textSize
• maxWidth
• padding
• block

UnderlineTabs

• size
• overlapBorder
• inactiveColor
• activeColor
• hoverColor
• bold

---

Once we have consensus about these changes, we'll need to do some cross-browser testing, then we can release. There is no huge hurry on this; but it'd be a good idea to finish up these changes within the next few weeks so the repo doesn't stagnate and we're unable to release bug fixes.

[davidtheclark]

I'd like to release these features this week, so if anybody wants to take a look, now's the time.

[samanpwbb]

I had a look through the code + test cases app. I like the direction! Here's some specific feedback:

• button – mostly like this component (the semantic variant prop is great), but uneasy about the default height of 42 – we don't have an h42 so aligning to the button will be awkward in some cases.
• popover – the coloring prop doesn't feel quite right. Should coloring be variant, and should we introduce semantic meaning to each color variant?
• tooltip – textSize prop uses abbreviations instead of words – which is inconsistent. Also, consider variant instead of coloring here too?
• heading – This component is in the spirit of where i'd love to see us take this library. Variants are clear and meaningful, props interface is nice and simple.
• icontext - spacing prop name sounds a little ambiguous. Unclear if it's vertical space, horizontal space, ect. Could this be called gapWidth?
• sizing props like padding: modal has only large or none, popover has small, medium, or none. Is the intent to make each variant always mean the same thing across any component (large is always 36, medium always 12, ect)? Seems like that is mostly true, with some exceptions in button where you're setting different width variants based on size variants. I think it'd be nice for this kind of predicability across the library to exist if possible.
• Many components still have theme props (control* components, ect.) – is the plan to get around to moving everything away from using theme props? In general, it's unclear to me what state the repo is in right now – feels like it's like 50% of the way through a refactor.

[davidtheclark]

Thanks for the notes, @samanpwbb. You're right that form components have not been changed. I figure that can come later. I like your points — I think we might need a little more discussion to decide how to act on them.

uneasy about the default height of 42

This is our current pattern. I'd be happy to change this if we have another button style that we want to rally around as the default. This is also something we could change later.

popover – the coloring prop doesn't feel quite right. Should coloring be variant, and should we introduce semantic meaning to each color variant?

Yeah, it is a weird one. However, I don't think there is any semantic distinction between dark & light; and it's not quite like the other variants because it only sets one value (for now). If you or anyone sees a pattern we could codify, though, we should give it a try.

icontext - spacing prop name sounds a little ambiguous. Unclear if it's vertical space, horizontal space, ect. Could this be called gapWidth?

Sure. Maybe just gap?

Is the intent to make each variant always mean the same thing across any component (large is always 36, medium always 12, ect)?

No, that wasn't the intent. My thinking was that those size words would always be relative to the component itself. In fact, within Button the padding value will have a different effect depending on your size. Modal only has padding large or none because we don't have a standard modal style that needs another padding value. I'm not sure predictability is as important a goal here as standardization, prescribing specific styles — and I worry that tying size words to number values would be a move away from standardization, towards Assembly-style flexibility. Open to suggestions about any ways we could get the best of both.

Any of those responses trigger an idea?

[samanpwbb]

This is our current pattern. I'd be happy to change this if we have another button style that we want to rally around as the default. This is also something we could change later.

Right now button heights are: 42 for large, 30 for medium. I think these metrics could work: the idea is that buttons tend to be a little taller than everything else, which in practice looks nice most of the time. Lets try it out... The perfectionist in me would like height that are both aligned to existing classes and height/2 is aligned to an existing class (like 36 & 24) but I can get over this.

I'm not sure predictability is as important a goal here as standardization, prescribing specific styles — and I worry that tying size words to number values would be a move away from standardization, towards Assembly-style flexibility.

Yeah, makes sense. I still find it a little weird that one component has either large or none, while another has none, small, and medium options. I just have a minor suggestion in that case: use a system for naming size variants:

• Two: props are none or large
• Three: props are none, small, large
• Four: props are none, small, medium, large

Yeah, it is a weird one. However, I don't think there is any semantic distinction between dark & light; and it's not quite like the other variants because it only sets one value (for now).

In Studio at least, we use dark popovers for contextual information, when we want content to have some separation from the rest of the app, and we use white popovers for content that should feel like part of the UI. Of course, the reverse would be true if an app is predominantly dark. I would be in favor of: all our UIs should be predominantly light, and dark popovers should be reserved for reference info / onboarding / ect. So, the variants would be ui or standard, and then info or something. This is maybe too opinionated, but curious to hear your thoughts.

[davidtheclark]

Interesting idea about the size-variant naming scheme. My one reservation is that I can imagine cases where we'd know we have, say, only one option at the moment and the relevant size is clearly not "large", and if we were to add more sizes in the future they would be larger than the current one. Then we'd have to make a breaking change where what used to be called "large" is now called "small" or "medium". So I was trying to base the word choice on guesses about where we might want room to add more options in the future. How do you think that concern weighs in?

In Studio at least, we use dark popovers for contextual information, when we want content to have some separation from the rest of the app, and we use white popovers for content that should feel like part of the UI.

In Studio are all tooltips are dark popovers, and all dark popovers tooltips? Are there any cases where you'd use a light tooltip? Your description of the semantics sounds like what tooltips. I'd be fine with dark being the the background for every tooltip — but we'd still need to expose it on the Popover itself, and PopoverTrigger. I guess we could have a variant prop with tooltip | tooltipWarning | standard?

[samanpwbb]

So I was trying to base the word choice on guesses about where we might want room to add more options in the future. How do you think that concern weighs in?

Yeah, I understand this, makes sense. I'm trying to think about usability: as a user, you want to be able to reliably "guess" what the possible values are based on some patterns you've learned, without needing to look up docs. I don't think my suggestion is better than yours in that respect (in both cases users need to guess – there's no way to know how many different size options exist without looking at docs), so 👍 for sticking with what you have.

In Studio are all tooltips are dark popovers, and all dark popovers tooltips? Are there any cases where you'd use a light tooltip?

We don't have any light tooltips. We have dark popovers though, in onboarding:

[davidtheclark]

I don't know what to do about the popover coloring 🤷‍♂️

@angel could you give us some insight into whether you think Account or Admin or other apps should/will be following Studio's pattern here?

I still can't think of any prop name more direct and meaningful than coloring. If we could add variant, it would be a higher-level prop that would incorporate coloring and spacing and maybe other styling props.

[ahelkit]

@davidtheclark in accounts we use this light tool tip, but no other popovers:

I don't really see this as a pattern that accounts will be using. I see us using more tooltips without buttons.

[samanpwbb]

I still can't think of any prop name more direct and meaningful than coloring.

Thats fine for now – eventually I do believe it will be in our benefit to move towards style variants with semantic meaning.