RFC: how to handle tokens for existing CSS-in-JS approach in v0

[mnajdova]

BREAKING CHANGE

Removed the following properties from the AttachmentAction component:

• text
• iconOnly
• circular
• size
• fluid
• inverted

If any of these is needed we can add it after it is requested

Tokens structure

Problem description

Variables (tokens) are a way of customizing styles in the Fluent components. For example:

<Attachment
  header="Photo.jpg"
  action={{
    icon: <CloseIcon />,
    onClick: () => alert("'X' is clicked!"),
    title: 'Close',
  }}
  variables={{backgroundColor: 'lightblue'}}
/>

will update the attachment to have blue background.

For customizing the root component itself we don't have any problems, but for the slots things are starting to get more interesting.

Let's see the next example

<Attachment
  header="Photo.jpg"
  variables={{backgroundColor: 'lightblue', actionBackgroundColor: 'lightgreen'}}
  action={{
    icon: <CloseIcon />,
    title: 'Close',
  }}
/>

We would expect that the following will happen:

But that's not true unfortunately :(

However this code, will produce the prev look:

<Attachment
  header="Photo.jpg"
  variables={{backgroundColor: 'lightblue'}}
  action={{
    style: { backgroundColor:'lightgreen' },
    icon: <CloseIcon />,
    title: 'Close',
  }}
/>

Let's see why this is happening

The AttachmentAction which is used for the action slot, is composing the Button component. The Button component has it's own styles and variables. If we just use the Attachment's variables for the AttachmentAction, we will have a problem, because we will have variables name collision - both the Button and the Attachment have backgroundColor variable, that's why the last code snippet updated the background color for the action.

How can we solve this?

Proposed solution

When composing components for our Fluent slots, we should always override the styles for the base component. That means that we will always need to write the styles from scratch, but it will allow us to use the parent's components variables and pick the correct variables which are intended for the slot (for example set the background color to be the actionBackgroundColor variable in the AttachmentAction's styles). This will also save us from unpredictable regressions, when we are updating the styles for one component, but that is doing changes all over the codebases, where the styles of this component may have been used/composed.

In addition to this, it will mean that we won't need to have variables for the slots components. This will be possible because all variables for a component will be defined on the root level, where we can structure the variables names to always start with the slot name, or we can have object structure for the slots.

Option 1

const attachmentVariables = siteVars => ({
    // variables for the root slot
    rootBackgroundColor: siteVars.brand,
    rootColor: siteVars.neutral,
    
    // variables for the action slot
    actionBackgroundColor: siteVars.neutral,
    actionColor: siteVars.brand,
    
    // variables for the icon slot
    iconSize: '2rem',
});

Option 2

const attachmentVariables = siteVars => ({
    // variables for the root slot
    root: {
        backgroundColor: siteVars.brand,
        color: siteVars.neutral,
    },
    
    // variables for the action slot
    action: {
        backgroundColor: siteVars.neutral,
        color: siteVars.brand,
    },
    
    // variables for the icon slot
    icon: { 
        size: '2rem'
    },
});

Apart from this, we have to share the variables between the parent and children components via context, so that the variables usage in the parent component can trigger updates in the styles of the children components inside.

I am leaning towards #Option1.1 as it is closed to what we have, plus it will preserve the flat variables structure, which is closed to the CSS variables

[msft-github-bot]

Perf Analysis

No significant results to display.

All results

Scenario	Master Ticks	PR Ticks	Iterations	Status
BaseButton	827	801	5000
Checkbox	1560	1534	5000
CheckboxBase	1182	1286	5000
ChoiceGroup	4673	4649	5000
ComboBox	902	946	1000
CommandBar	7500	7568	1000
ContextualMenu	15361	14850	1000
DefaultButton	978	1027	5000
DetailsRow	3291	3224	5000
DetailsRow (fast icons)	3348	3260	5000
DetailsRow without styles	3097	3025	5000
Dialog	1405	1420	1000
DocumentCardTitle with truncation	1597	1662	1000
Dropdown	2346	2357	5000
FocusZone	1606	1595	5000
IconButton	1651	1713	5000
Label	273	288	5000
Link	437	422	5000
MenuButton	1336	1421	5000
Nav	3062	2960	1000
Panel	1325	1415	1000
Persona	787	806	1000
Pivot	1240	1206	1000
PrimaryButton	1153	1201	5000
SearchBox	1276	1254	5000
Slider	1475	1395	5000
Spinner	360	391	5000
SplitButton	2971	2942	5000
Stack	437	447	5000
Stack with Intrinsic children	1079	1057	5000
Stack with Text children	3940	3876	5000
TagPicker	2615	2689	5000
Text	346	355	5000
TextField	1338	1294	5000
Toggle	869	810	5000
button	56	64	5000

Perf Analysis (Fluent)

⚠️ 1 potential perf regressions detected

Potential regressions comparing to master

Scenario	Current PR Ticks	Baseline Ticks	Ratio	Regression Analysis
AttachmentMinimalPerf.default	147	162	0.91:1	analysis

Perf comparison

Status	Scenario	Fluent TPI	Fabric TPI	Ratio	Iterations	Ticks
🔧	Avatar.Fluent	0.47	0.45	1.04:1	2000	947
🦄	Button.Fluent	0.1	0.19	0.53:1	5000	509
🔧	Checkbox.Fluent	0.65	0.35	1.86:1	1000	653
🔧	Dialog.Fluent	0.34	0.19	1.79:1	5000	1698
🔧	Dropdown.Fluent	3.22	0.43	7.49:1	1000	3222
🔧	Icon.Fluent	0.14	0.05	2.8:1	5000	708
🦄	Image.Fluent	0.07	0.11	0.64:1	5000	333
🔧	Slider.Fluent	1.4	0.35	4:1	1000	1404
🔧	Text.Fluent	0.07	0.02	3.5:1	5000	335
🦄	Tooltip.Fluent	0.09	18.23	0:1	5000	451

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
DividerMinimalPerf.default	345	310	1.11:1
Icon.Fluent	708	644	1.1:1
TextMinimalPerf.default	342	316	1.08:1
ChatDuplicateMessagesPerf.default	425	396	1.07:1
DropdownMinimalPerf.default	3488	3264	1.07:1
HeaderMinimalPerf.default	499	465	1.07:1
MenuButtonMinimalPerf.default	1555	1468	1.06:1
ProviderMinimalPerf.default	667	627	1.06:1
RadioGroupMinimalPerf.default	541	511	1.06:1
IconMinimalPerf.default	655	618	1.06:1
ButtonMinimalPerf.default	160	153	1.05:1
HierarchicalTreeMinimalPerf.default	954	907	1.05:1
ToolbarMinimalPerf.default	827	789	1.05:1
Text.Fluent	335	319	1.05:1
DropdownManyItemsPerf.default	1362	1306	1.04:1
ItemLayoutMinimalPerf.default	1625	1559	1.04:1
TextAreaMinimalPerf.default	442	423	1.04:1
Avatar.Fluent	947	907	1.04:1
Checkbox.Fluent	653	626	1.04:1
AvatarMinimalPerf.default	508	492	1.03:1
ChatMinimalPerf.default	631	611	1.03:1
FormMinimalPerf.default	732	709	1.03:1
LabelMinimalPerf.default	386	375	1.03:1
ProviderMergeThemesPerf.default	1670	1614	1.03:1
TreeWith60ListItems.default	229	223	1.03:1
ListNestedPerf.default	874	860	1.02:1
TooltipMinimalPerf.default	696	683	1.02:1
VideoMinimalPerf.default	590	579	1.02:1
CheckboxMinimalPerf.default	2877	2839	1.01:1
GridMinimalPerf.default	626	621	1.01:1
LoaderMinimalPerf.default	733	728	1.01:1
SliderMinimalPerf.default	1397	1377	1.01:1
StatusMinimalPerf.default	647	642	1.01:1
Dialog.Fluent	1698	1684	1.01:1
Image.Fluent	333	329	1.01:1
AnimationMinimalPerf.default	615	613	1:1
ButtonSlotsPerf.default	598	596	1:1
HeaderSlotsPerf.default	1445	1439	1:1
LayoutMinimalPerf.default	524	523	1:1
RefMinimalPerf.default	195	195	1:1
CustomToolbarPrototype.default	3690	3694	1:1
AccordionMinimalPerf.default	135	137	0.99:1
AlertMinimalPerf.default	282	285	0.99:1
BoxMinimalPerf.default	309	311	0.99:1
EmbedMinimalPerf.default	1947	1976	0.99:1
FlexMinimalPerf.default	264	266	0.99:1
PopupMinimalPerf.default	244	247	0.99:1
ReactionMinimalPerf.default	1811	1837	0.99:1
InputMinimalPerf.default	981	1003	0.98:1
ListCommonPerf.default	936	956	0.98:1
SplitButtonMinimalPerf.default	3210	3265	0.98:1
TableMinimalPerf.default	355	361	0.98:1
TreeMinimalPerf.default	1111	1139	0.98:1
Button.Fluent	509	520	0.98:1
AttachmentSlotsPerf.default	1072	1103	0.97:1
CarouselMinimalPerf.default	466	479	0.97:1
ImageMinimalPerf.default	336	347	0.97:1
MenuMinimalPerf.default	1780	1843	0.97:1
Slider.Fluent	1404	1443	0.97:1
Tooltip.Fluent	451	463	0.97:1
DialogMinimalPerf.default	1711	1778	0.96:1
PortalMinimalPerf.default	292	304	0.96:1
SegmentMinimalPerf.default	911	953	0.96:1
ChatWithPopoverPerf.default	559	587	0.95:1
ListWith60ListItems.default	1105	1167	0.95:1
Dropdown.Fluent	3222	3389	0.95:1
CardMinimalPerf.default	508	539	0.94:1
ListMinimalPerf.default	419	449	0.93:1

[assuncaocharles]

IMO the 2nd option seems more organised and natural to think, I would guess also easy to maintain since the parts are well separated slot by slot?

[layershifter]

My vote is for option 1 as it's closer to CSS variables. Option 2 existed at some point (microsoft/fluent-ui-react#207 ??) and we removed it, however we still have some nested variables for colors 😭

---

Q1: How to pass variables down?

I see two options: via React Context or via props (breaks caching)?

Q2: Theme definition?

Currently we have attachmentBodyVariables.ts that contain only reexports are we going to solve it somehow?

export { default } from './attachmentVariables';

Q3: Enforcing naming rules

It looks that all will fit pattern SLOT_(PROP_VALUE)_CSS_VALUE. Should we enforce it somehow? Via ESLint rule?

Q4: Collection components

Can you please add an example for collection components? How names will look there? menuItemContentOpenBackgroundColor? What will be root in this case?

[size-auditor]

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: 31404935a5e8287ea7e5fdb491c1deb96338b710 (build)

[dzearing]

I also intuitively went with option 1 in my prototyping.

[layershifter]

Technically this is a breaking change, but as this was never released we are good

[layershifter]

At least let's move this out of render scope and add a comment to do something with as obviously this about is not a key to the bright future

[mnajdova]

Moved and added comment. I agree that we should wither fix mergeComponentVariables or provide similar utility