feat(makeStyles): add shorthand functions by layershifter · Pull Request #20628 · microsoft/fluentui

layershifter · 2021-11-16T17:07:33Z

Pull request checklist

Addresses an existing issue: Partially implements RFC: Do not allow CSS shorthands in makeStyles() calls #20573
Include a change request file using $ yarn change

Description of changes

This PR is a first step in implementation of #20573, you can check draft PR (#20539) to see how it will be used.

This PR partially implements RFC #20573 and adds shorthand functions (other changes will come in separate PRs 💪). These functions are designed to simplify usage of CSS longhands, for example 👇

makeStyles({
  rootA: {
    ...shorthands.border("2px", "solid", "red")
  },
  rootB: {
    borderBottomWidth: "2px",
    borderLeftWidth: "2px",
    borderRightWidth: "2px",
    borderTopWidth: "2px",
    borderBottomStyle: "solid",
    borderLeftStyle: "solid",
    borderRightStyle: "solid",
    borderTopStyle: "solid",
    borderBottomColor: "red",
    borderLeftColor: "red",
    borderRightColor: "red",
    borderTopColor: "red"
  }
});

There functions are exported from @fluentui/make-styles package and re-exported in @fluentui/react-components (via @fluentui/react-make-styles:

import { shorthands } from "@fluentui/react-components";

makeStyles({
  root: {
    ...shorthands.border("2px", "solid", "red")
  }
});

Implemented functions

`border`, `borderBottom`, `borderLeft`, `borderTop`, `borderRight`

borderBottom("2px");
borderBottom("2px", "solid");
borderBottom("2px", "solid", "red");

⚠️ These functions have strict arguments order and do not support variable order of properties as CSS:

/* style */
border: solid;
/* width | style */
border: 2px dotted;
/* style | color */
border: outset #f33;

`borderColor`, `borderStyle`, `borderWidth`, `margin`, `padding`

borderColor("red");
borderColor("red", "blue");
borderColor("red", "blue", "green");
borderColor("red", "blue", "green", "yellow");
borderStyle("solid");
borderWidth("12px", "24px", "36px");
margin("2px", "4px", "8px");
padding("10px", "5px");

`borderRadius`

borderRadius("10px");
borderRadius("10px", "5%");
borderRadius("2px", "4px", "8px");
borderRadius("1px", 0, "3px", "4px");

⚠️ Conformant to CSS spec, does not support / syntax:

/* (first radius values) / top-left | top-right-and-bottom-left | bottom-right */
border-radius: 10px 5px 2em / 20px 25px 30%;
/* (first radius values) / top-left | top-right | bottom-right | bottom-left */
border-radius: 10px 5% / 20px 25em 30px 35em;

`gap`, `overflow`

gap("10px");
gap("10px", "5px");

overflow("hidden");
overflow("hidden", "scroll");

fabricteam · 2021-11-16T17:14:41Z

📊 Bundle size report

_{🤖 This report was generated against 8a820339f7f4084d8c46e97f06eebbeb18111397}

size-auditor · 2021-11-16T17:22:33Z

Asset size changes

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

Baseline commit: 66670cf47c716e284c7ac7ae5f25d1f77f3aced3 (build)

codesandbox-ci · 2021-11-16T17:38:55Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cc1f0bf:

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration

fabricteam · 2021-11-16T17:51:00Z

Perf Analysis (`@fluentui/react`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1129	1176	5000
BaseButton	mount	1133	1159	5000
Breadcrumb	mount	2953	2941	1000
ButtonNext	mount	729	699	5000
Checkbox	mount	1920	1880	5000
CheckboxBase	mount	1620	1625	5000
ChoiceGroup	mount	5494	5549	5000
ComboBox	mount	1195	1166	1000
CommandBar	mount	11054	11056	1000
ContextualMenu	mount	9173	9178	1000
DefaultButton	mount	1408	1459	5000
DetailsRow	mount	4296	4302	5000
DetailsRowFast	mount	4269	4232	5000
DetailsRowNoStyles	mount	4144	4107	5000
Dialog	mount	2919	2882	1000
DocumentCardTitle	mount	335	339	1000
Dropdown	mount	3713	3670	5000
FluentProviderNext	mount	4307	4317	5000
FluentProviderWithTheme	mount	359	376	10
FluentProviderWithTheme	virtual-rerender	249	262	10
FluentProviderWithTheme	virtual-rerender-with-unmount	450	433	10
FocusTrapZone	mount	2140	2022	5000
FocusZone	mount	2111	2070	5000
IconButton	mount	2115	2121	5000
Label	mount	539	540	5000
Layer	mount	3394	3493	5000
Link	mount	685	683	5000
MakeStyles	mount	2052	2063	50000
MenuButton	mount	1894	1822	5000
MessageBar	mount	2415	2252	5000
Nav	mount	3700	3874	1000
OverflowSet	mount	1367	1354	5000
Panel	mount	2792	2675	1000
Persona	mount	1075	1026	1000
Pivot	mount	1701	1653	1000
PrimaryButton	mount	1552	1595	5000
Rating	mount	8819	8751	5000
SearchBox	mount	1654	1639	5000
Shimmer	mount	2944	2996	5000
Slider	mount	2322	2301	5000
SpinButton	mount	5676	5686	5000
Spinner	mount	608	619	5000
SplitButton	mount	3643	3523	5000
Stack	mount	719	746	5000
StackWithIntrinsicChildren	mount	2059	2026	5000
StackWithTextChildren	mount	5421	5374	5000
SwatchColorPicker	mount	11727	11641	5000
TagPicker	mount	3035	2974	5000
TeachingBubble	mount	14064	14088	5000
Text	mount	632	651	5000
TextField	mount	1691	1669	5000
ThemeProvider	mount	1432	1381	5000
ThemeProvider	virtual-rerender	816	832	5000
ThemeProvider	virtual-rerender-with-unmount	2212	2193	5000
Toggle	mount	1042	1069	5000
buttonNative	mount	318	300	5000

Perf Analysis (`@fluentui/react-northstar`)

Perf tests with no regressions

Scenario	Current PR Ticks	Baseline Ticks	Ratio
AttachmentMinimalPerf.default	195	177	1.1:1
ImageMinimalPerf.default	447	407	1.1:1
LabelMinimalPerf.default	475	435	1.09:1
AlertMinimalPerf.default	333	310	1.07:1
ChatDuplicateMessagesPerf.default	355	333	1.07:1
TreeWith60ListItems.default	202	189	1.07:1
DropdownManyItemsPerf.default	811	765	1.06:1
FlexMinimalPerf.default	324	306	1.06:1
LayoutMinimalPerf.default	449	423	1.06:1
RefMinimalPerf.default	261	247	1.06:1
TreeMinimalPerf.default	937	887	1.06:1
ListWith60ListItems.default	740	703	1.05:1
ReactionMinimalPerf.default	429	409	1.05:1
SkeletonMinimalPerf.default	411	391	1.05:1
StatusMinimalPerf.default	784	748	1.05:1
AnimationMinimalPerf.default	467	450	1.04:1
ChatMinimalPerf.default	764	733	1.04:1
DividerMinimalPerf.default	432	414	1.04:1
FormMinimalPerf.default	469	449	1.04:1
HeaderMinimalPerf.default	419	403	1.04:1
LoaderMinimalPerf.default	760	733	1.04:1
MenuButtonMinimalPerf.default	1838	1759	1.04:1
ButtonSlotsPerf.default	619	603	1.03:1
MenuMinimalPerf.default	948	923	1.03:1
ProviderMergeThemesPerf.default	1793	1744	1.03:1
RadioGroupMinimalPerf.default	523	506	1.03:1
InputMinimalPerf.default	1408	1376	1.02:1
SegmentMinimalPerf.default	387	379	1.02:1
SplitButtonMinimalPerf.default	4771	4661	1.02:1
AvatarMinimalPerf.default	235	233	1.01:1
ChatWithPopoverPerf.default	411	407	1.01:1
EmbedMinimalPerf.default	4494	4466	1.01:1
ItemLayoutMinimalPerf.default	1347	1328	1.01:1
TableManyItemsPerf.default	2159	2132	1.01:1
TextMinimalPerf.default	386	382	1.01:1
CustomToolbarPrototype.default	4439	4412	1.01:1
BoxMinimalPerf.default	405	407	1:1
ButtonOverridesMissPerf.default	1874	1882	1:1
CheckboxMinimalPerf.default	2900	2909	1:1
HeaderSlotsPerf.default	861	860	1:1
ListCommonPerf.default	733	731	1:1
PortalMinimalPerf.default	202	203	1:1
ToolbarMinimalPerf.default	1070	1075	1:1
DialogMinimalPerf.default	843	850	0.99:1
SliderMinimalPerf.default	1818	1828	0.99:1
TextAreaMinimalPerf.default	580	586	0.99:1
AttachmentSlotsPerf.default	1156	1177	0.98:1
CarouselMinimalPerf.default	514	522	0.98:1
DropdownMinimalPerf.default	3167	3236	0.98:1
ListMinimalPerf.default	580	593	0.98:1
RosterPerf.default	1312	1343	0.98:1
PopupMinimalPerf.default	648	663	0.98:1
IconMinimalPerf.default	647	663	0.98:1
DatepickerMinimalPerf.default	6002	6159	0.97:1
TableMinimalPerf.default	451	466	0.97:1
AccordionMinimalPerf.default	171	178	0.96:1
ProviderMinimalPerf.default	1232	1283	0.96:1
TooltipMinimalPerf.default	1132	1182	0.96:1
VideoMinimalPerf.default	672	700	0.96:1
CardMinimalPerf.default	621	654	0.95:1
GridMinimalPerf.default	377	397	0.95:1
ListNestedPerf.default	632	666	0.95:1
ButtonMinimalPerf.default	197	211	0.93:1

miroslavstastny · 2021-11-22T15:04:16Z

packages/make-styles/src/types.ts

+
+export interface MakeStyles extends Omit<CSSProperties<MakeStylesCSSValue>, 'animationName'> {
  // TODO Questionable: how else would users target their own children?
  [key: string]: any; // eslint-disable-line @typescript-eslint/no-explicit-any


Would it make sense to improve the typing of the MakeStyles in the prototype as it is supposed to be the only guard against the shorthands? Nested objects must have the same restrictions recursively.

This is done in #20539 (giant PR 💥). I don't change typings in this PR as it only adds these functions.

packages/make-styles/src/shorthands/border.ts

packages/make-styles/src/shorthands/borderBottom.test.ts

packages/make-styles/src/shorthands/border.test.ts

ling1726 · 2021-11-23T12:45:01Z

packages/make-styles/src/shorthands/borderLeft.ts

+import { BorderColorProperty, BorderStyleProperty, BorderWidthProperty } from 'csstype';
+import { MakeStyles, MakeStylesCSSValue } from '../types';


Suggested change

import { BorderColorProperty, BorderStyleProperty, BorderWidthProperty } from 'csstype';

import { MakeStyles, MakeStylesCSSValue } from '../types';

import type { BorderColorProperty, BorderStyleProperty, BorderWidthProperty } from 'csstype';

import type { MakeStyles, MakeStylesCSSValue } from '../types';

I've seen this starting to become a thing in the repo, should we do it ?

Well, there is no ESLint rule around it ¯_(ツ)_/¯
But I will fix all imports 👍

@dzearing had a series of PRs to change everything possible to import type, but some of them didn't get merged. It looks like he also added @typescript-eslint/eslint-plugin rules consistent-type-imports and consistent-type-exports but didn't get around to enabling them here.

The import rule has a clever way to avoid needing type information, which in theory should prevent it from being terribly expensive to run, but hard to say without measuring it. The exports rule does require type info and therefore could be slower.

ling1726 · 2021-11-23T12:58:46Z

packages/make-styles/src/shorthands/borderLeft.ts

+ * See https://developer.mozilla.org/en-US/docs/Web/CSS/border-left
+ */
+export function borderLeft(
+  ...values: [BorderWidthProperty<MakeStylesCSSValue>, BorderStyleProperty?, BorderColorProperty?]


wow that's a really neat overload trick 💪

ling1726 · 2021-11-23T12:59:22Z

packages/make-styles/src/shorthands/borderRadius.test.ts

+import { borderRadius } from './borderRadius';
+
+describe('borderRadius', () => {
+  it('expands to an object for a single value', () => {


nit: single 2 3 values for test naming seem kinda confusing

Suggestions?

I would go with the type of the values that are passed. For the specific case of border-radius:

for a given length

for a top-left-and-bottom-right length and a top-right-and-bottom-left percentage

for a top-left, top-right-and-bottom-left and bottom-right length

for a top-left, top-right, bottom-right and bottom-left length

This suggestion could be taken for the other tests too.

@andrefcdias applied your suggestion in cc1f0bf 👍

packages/make-styles/src/shorthands/overflow.ts

packages/make-styles/src/shorthands/borderBottom.ts

packages/make-styles/src/shorthands/borderLeft.ts

packages/make-styles/src/shorthands/borderRight.ts

packages/make-styles/src/shorthands/borderTop.ts

Co-authored-by: ling1726 <lingfan.gao@microsoft.com>

theerebuss · 2021-11-23T16:36:09Z

packages/make-styles/src/shorthands/borderRadius.test.ts

+import { borderRadius } from './borderRadius';
+
+describe('borderRadius', () => {
+  it('expands to an object for a single value', () => {


I would go with the type of the values that are passed. For the specific case of border-radius:

for a given length

for a top-left-and-bottom-right length and a top-right-and-bottom-left percentage

for a top-left, top-right-and-bottom-left and bottom-right length

for a top-left, top-right, bottom-right and bottom-left length

This suggestion could be taken for the other tests too.

…eat/mk-macros

* feat(makeStyles): add macro functions * Change files * restore config change * fix exports * fix import * rename to shorthands * revert change * add comment * add overflow & gap * Apply suggestions from code review Co-authored-by: ling1726 <lingfan.gao@microsoft.com> * use "import type" * add export * Change files * update API file * update description it tests Co-authored-by: ling1726 <lingfan.gao@microsoft.com>

layershifter added 2 commits November 16, 2021 18:06

feat(makeStyles): add macro functions

08da4da

Change files

9f64e3c

msft-fluent-ui-bot assigned khmakoto Nov 16, 2021

layershifter changed the title ~~Feat/mk macros~~ feat(makeStyles): add macro functions Nov 16, 2021

restore config change

db405d1

fix exports

510e05f

fix import

601934d

layershifter changed the title ~~feat(makeStyles): add macro functions~~ feat(makeStyles): add shorthand functions Nov 18, 2021

rename to shorthands

d64124d

layershifter added Fluent UI react-components (v9) Area: JS Styling labels Nov 22, 2021

revert change

3f20cc9

layershifter mentioned this pull request Nov 22, 2021

API extractor prevents usage of re-exports 💥 #20694

Open

layershifter added 2 commits November 22, 2021 11:39

add comment

8b3f544

add overflow & gap

7bca4e1

miroslavstastny reviewed Nov 22, 2021

View reviewed changes

layershifter marked this pull request as ready for review November 23, 2021 09:42

layershifter requested a review from a team as a code owner November 23, 2021 09:42

layershifter requested review from a team and GeoffCoxMSFT and removed request for a team November 23, 2021 09:42