Should we include 0 in our spacing token scale?

[lgriffee]

We currently have a --p-space-0 token in our spacing scale. And our coverage site fails any spacing category property (e.g. margin, padding) lines that use 0 instead of a token.

Do we want to:

1. Keep failing lines with 0 + use the token (e.g. padding: var(--p-space-0) var(--p-space-4))
2. Start passing lines with 0 + remove this token + use 0 (e.g. padding: 0 var(--p-space-4))
3. Some other combination of failing/passing lines and keeping/removing the token

[aveline]

2️⃣

At this time, we don't include 0 for border-radius. Regardless of what we decide, we should be consistent across token groups on whether we include a 0 token.

[chloerice]

My thought is that the answer depends on our goals for tracking moving forward (after mainline is reached):

• If we want to holistically measure and understand the use of spacing values, and especially if the majority of spacing values in web today are zero, then I think it does make sense to have a zero value.
• If we don't plan to track token usage beyond the work that we're doing to reach the mainline goal, then all having a zero value for spacing would do (imo) is validate the need for the Box component (which could also be valuable data for framing the decision to take the system in that direction, but we don't need a token value to collect that data).

[aaronccasanova]

If we want to holistically measure and understand the use of spacing values[...]

Interesting point! Yeah, removing the zero token would somewhat omit data points on spacing as a concept. Right now we can easily search and evaluate how --p-space-0 is used without scripting.

[alex-page]

I agree with 2. Zero will always be zero. Even if we add a token for it. My gut feeling is we should keep this simple and encourage the usage of the number zero and not provide a token. We could even document the number (not the token) across our token docs.

[SeanyB]

I don't have a strong opinion on this, and I'm happy to defer to this group.

But if we choose option 2, I want to confirm next steps.

Would this be the total set of work to achieve consistency inside Polaris tokens and our coverage analysis?:

• Remove --p-space-0 token
• Remove --p-duration-0 token
• Update coverage analysis to pass 0 values for Space and Motion

[alex-page]

Yes, also shape (border-radius).

[tjonx]

null values or 0 values are system-agnostic constants and shouldn't require any additional mapping IMO.
2️⃣

Side note: why are we specifying size with a token that's called space? All spaces have sizes, but not all sizes are spaces.

[SeanyB]

@tjonx - I think the space token name is correct because the space tokens specify the size of spaces in the UI (gaps between objects). Size is a more general term. For example, --p-border-width-1 specifies the size of the border width, but we wouldn't use "size" in the token name.

[aaronccasanova]

After researching other systems and reviewing the current responses I think I'm coming around to option 2️⃣

I originally preferred zero tokens for things like:

• Style resets in composite tokens
• Intuitive programmatic logic (e.g. <Box space={space} /> --p-space-${props.space})
• Establishing a convention that zero means zero (i.e. I noticed systems using token-0, token-00, token-000 as negative lengths)

That said, I'm still impartial on whether or not we continue to support zero length tokens 1️⃣ 🤷 2️⃣

[heyjoethomas]

2. 0 should not be a token.

[kyledurand]

I don't see any value in tokenizing 0 and would be slightly annoyed with a linter asking me to replace my margin: 0 with margin: var(--p-space-0). Especially if it's just for the sake of our coverage numbers

Option 2 📈

[sam-b-rose]

I am in favor of option 2️⃣ . It keeps it simple by providing what is only core to the system, not the kitchen sink of spacing values.

This is similar to having a token for the color black named "black". Zero will always be 0️⃣ 👍

It may seem tidy to have a 0 token but it isn't a lever we'd ever want to adjust within the system.

[alex-page]

Going to write the other side of this as most folks are leaning towards option two.

If we had a token for 0 we could easily count usage, document the value with our tokens without anything special and include it with tooling without any workarounds. Teams are taught to use the tokens and only the tokens. Which might be simpler then explaining that 0 is a number value and there are also tokens for other values.

[lgriffee]

Thanks for weighing in everyone! 🙏🏻✨🎉❤️ We will move forward with option 2️⃣ based on our team's responses. Option 2️⃣ will provide the best experience for our developers (and option 1️⃣ doesn’t provide enough value to outweigh this). If we run into issues with this approach in the future we can always discuss this further as a team. 💬

[lgriffee]

🚨 Reopening Question 🚨

Yesterday, as I was trying to remove our --p-space-0 and --p-duration-0 tokens in our v11 branch, @aaronccasanova , @aveline and I ran into some consequences that I wanted to discuss as team before moving forward.

If we remove zero tokens from our scales it will:

1. Force developers to omit properties when their intention is to apply zero values
   ⚠️This is potentially unintuitive based on developer's existing knowledge of CSS (e.g. how they apply zero to properties).

   - <Box padding={input} />
   + <Box padding={input === 0 ? undefined : input} />

   - <div style={{ padding: `var(--p-spacing-{input})` }} />
   + <div style={input === 0 ? undefined : { padding: `var(--p-spacing-{input})` }} />

2. Prevent developers from being able to remove default styles
   ⚠️Which was brought up in this team discussion about the Bleed component.

   <Bleed bottom="0" /> // Unable to remove default 🙅🏻‍♀️

3. Require developers to set each shorthand property when their intention is to reset a single value

   - <Box padding="2" paddingBlockEnd="0" />
   + <Box paddingBlockStart="2" paddingInlineStart="2" paddingInlineEnd="2" />

4. Removes our ability to apply safeguards for unique browser behavior
   ⚠️ Unitless 0 values are invalid for CSS animations. This may not be commonly known and tokenizing is an additional layer of protection.

.class {
    transition: opacity 0; // <time> values must have units (0ms or 0s) 🙅🏻‍♀️
}

Moving Forward

• Are we still okay with moving forward with removing the zero tokens given these trade-offs?
• Can we move forward with zero tokens but not enforce them as coverage failures (allowing us to expose them through components without the hard requirement of their usage as tokens)?

[sam-b-rose]

I am comfortable moving forward with removing the zero tokens.

Most challenges here are component-related. We can adjust the components to achieve the desired developer experience. I wouldn't want intrinsic component behavior to determine our desired token structure or component prop API.

Regarding the safeguards, I would think this is something linting would be able to help prevent. Maybe a hot take, but the tokens are more of guardrails rather than safeguards (ex: use these subset of values for a consistent UX; not use these values to help you write valid CSS).

Based on our previous discussion and the context here, the pros are worth the trade-offs in my opinion 👍

[jesstelford]

tl;dr: I'm in favour of removing the zero tokens.

1. Force developers to omit properties when their intention is to apply zero values

In the <Box> example, could you expand on why the dev might have to pass in undefined? I would expect that they could simply pass in 0 as the value, no? Afaik, our React API does not match our token names, and we're in control of how the Box component handles prop values.

The <div> example is interesting! I hadn't considered devs might be using template strings to construct CSS var statements in JS... It's like a custom lo-fi CSS-in-JS! In this case, I'd recommend the dev to just use the <Box> component, which will handle it as described above 👍

2. Prevent developers from being able to remove default styles

After reading the linked discussion, it seems to talk more about the case when a prop isn't passed at all (ie; value is undefined), not when the value is 0 - do you mind elaborating on how passing a 0 value prevents devs from removing defaults? (btw; I'm firmly in the camp that none of our components should come with default values which affect the positioning of the element within its parent, so the particular <Bleed> case is trivial: don't have defaults. See: Margin considered harmful)

3. Require developers to set each shorthand property when their intention is to reset a single value

Oh, interesting! The <Box padding="2" paddingBlockEnd="0" /> implies that props are applied in a particular order (shorthand first, then longhand), and that is a feature devs are relying on!? That fact doesn't appear to be documented, and I would even consider that a bug!

IMO the types should be structured in a way that a dev must provide either the shorthand only, or all long hand values: React props are not CSS, and trying to emulate the cascade in JS is a slippery slope of bugs.

4. Removes our ability to apply safeguards for unique browser behavior

I agree with @samrose3 here: We're providing tokens, not trying to teach devs CSS. I would vote for adding a note to our docs pointing to the CSS spec docs, but we don't need to do anything special here.

[aveline]

I'm firmly in the camp that none of our components should come with default values which affect the positioning of the element within its parent, so the particular case is trivial: don't have defaults.

We have been having some back and forth on defaults for the Bleed component. Perhaps worth creating a separate discussion so you can weigh in there @jesstelford.

implies that props are applied in a particular order (shorthand first, then longhand)

Not the case, the prop order doesn't matter. But we are currently allowing passing in both shorthand and individual props where individual will override shorthand. If you have strong opinions on only allowing one or the other, it would be worth starting a separate discussion.

I don't have a strong opinion on keeping or removing the zero tokens, but am following this closely as a change will impact our current work on the Layout components.

[tjonx]

4. Removes our ability to apply safeguards for unique browser behavior
⚠️ Unitless 0 values are invalid for CSS animations. This may not be commonly known and tokenizing is an additional layer of protection.

.class {
    transition: opacity 0; // <time> values must have units (0ms or 0s) 🙅🏻‍♀️
}

In this case, or any case where the zero unit is a unit of time, I'd question why any transition or animation should be used in the first place. (That goes hand-in-hand with the 'not teaching CSS' argument).

[lgriffee]

Thanks for weighing in again @samrose3, @jesstelford , @aveline, and @tjonx ! 🙏🏻✨🎉❤️ Based on y’alls responses we will continue to move forward with option 2️⃣ (removing the tokens)!

[lgriffee]

🚨 DECISION UPDATE 🚨

We are going to keep our zero tokens, but not enforce unitless zeros in stylelint-polaris

Decision Makers: @alex-page , @aaronccasanova , and @lgriffee

---

While working to remove the zero tokens in our system (--p-space-0 and --p-duration-0) it’s become apparent that the lift required to remove these tokens is too large/complex to justify removing them.

In the case of --p-space-0 this token is valuable in our components for setting responsive styles and would otherwise require complex logic/workarounds to implement. Keeping this token does not mean that we will enforce its usage in place of unitless zero values with stylelint-polaris. We can have the benefit of using this token in our layout components without forcing developers to use it anywhere else.

In the case of --p-duration-0, this token is valuable because units cannot be omitted from data types (in other words, 0 is invalid and does not represent 0s or 0ms). Without this token, we don’t have an edge case free way to get stylelint-polaris to ignore 0ms or 0s while flagging all other non-token time values.