feat(react-card): add `orientation` prop by theerebuss · Pull Request #23037 · microsoft/fluentui

theerebuss · 2022-05-17T15:48:17Z

New Behavior

Added the orientation property which will control:

flex container direction
center alignment of content for horizontal layouts

The 6 changes in Screener are due to the change in the viewport size, so they're expected.
#23037 (comment)

I plan on refactoring these stories in the future to use design templates, so for now I'm keeping changes simple to facilitate reviews.

Related Issue(s)

#19336

codesandbox-ci · 2022-05-17T15:56:17Z

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 203dbb6:

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

fabricteam · 2022-05-18T14:58:13Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`react-card` Card - All	`61.945 kB` `17.662 kB`	`63.222 kB` `17.946 kB`	`1.277 kB` `284 B`
`react-card` Card	`57.23 kB` `16.48 kB`	`58.378 kB` `16.819 kB`	`1.148 kB` `339 B`
`react-card` CardPreview	`7.709 kB` `3.286 kB`	`7.838 kB` `3.316 kB`	`129 B` `30 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-card` CardFooter	`7.737 kB` `3.264 kB`
`react-card` CardHeader	`9.302 kB` `3.779 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`183.617 kB` `50.924 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.988 kB` `11.108 kB`

_{🤖 This report was generated against 143f4ac9fb4eef165e4975bcf3b113bc00680e03}

size-auditor · 2022-05-18T15:00:35Z

Asset size changes

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

Baseline commit: 143f4ac9fb4eef165e4975bcf3b113bc00680e03 (build)

fabricteam · 2022-05-18T15:02:53Z

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

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	883	900	5000
Button	mount	566	569	5000
FluentProvider	mount	1847	1861	5000
FluentProviderWithTheme	mount	256	292	10
FluentProviderWithTheme	virtual-rerender	222	232	10
FluentProviderWithTheme	virtual-rerender-with-unmount	304	305	10
MakeStyles	mount	1605	1593	50000

theerebuss · 2022-06-02T09:26:38Z

+      marginTop: `calc(var(${cardCSSVars.cardSizeVar}) * -1)`,
+      marginBottom: `calc(var(${cardCSSVars.cardSizeVar}) * -1)`,
+    },
+    [`> :not([aria-hidden="true"]):first-of-type.${cardPreviewClassNames.root}`]: {


Unfortunate necessity due to Tabster's injected elements

Im usually against adding comments to codebase, but I think this is the exception. can we document this pls?

I think that's warranted 😓 Will do 👍🏻

I went ahead and added a little explanation to the other styles too. Let me know if you would change something.

all good lets get this in!

I don't have any concrete numbers I can share here or guidance on what to do instead but I want to share some potential issues with this selector.

Attribute selectors and pseudo-classes are some of the slowest selectors: source. Sadly it's difficult to substantiate this with numbers as devtools have dropped CSS selector profilers :(

Is it at all possible to refactor this selector to, say, drop the attribute selector? Does Tabster apply classes to its hidden elements? Could it be updated to if not?

thanks for reaching out @spmonahan ! The thing with Tabster DOM insertions is it can break at any time (which happened already). IMO would be best if tabster could expose a hook (css selector?) as part of its public API to give use solid contract. Can we reach out to folks @andrefcdias ? WDYT

Thank you for sharing your concerns, @spmonahan. I agree that this approach is not ideal, but please keep in mind that there will be some work put into optimization after all implementation is finished as not to keep delaying the implementation.
As @Hotell said, Tabster updates tends to be breaking and this was the lesser evil approach.
I'll keep this in mind for the future before finalizing Card.

theerebuss · 2022-06-02T09:28:07Z

+    [`> .${cardHeaderClassNames.root}, > .${cardFooterClassNames.root}`]: {
+      flexShrink: 0,
+    },


Ensuring header and footer won't go below the required sizes.

theerebuss · 2022-06-02T09:28:42Z

+    [`> :not(.${cardPreviewClassNames.root}):not(.${cardHeaderClassNames.root}):not(.${cardFooterClassNames.root})`]: {
+      flexGrow: 1,
+    },


Allow content to grow before Card components.

theerebuss · 2022-06-02T09:29:26Z


    '> *': {
      display: 'block',
+      height: '100%',


Now necessary to adjust CardPreview's content for horizontal cards.

theerebuss · 2022-06-02T09:30:54Z

+        <Card size="small" orientation="horizontal">
+          <CardPreview className={styles.horizontalPreview}>
+            <img src={Logo} alt="company logo template" />
+          </CardPreview>
+          <CardHeader
+            image={<Avatar badge={{ status: 'available' }} image={{ src: avatarElviaURL }} />}
+            header={
+              <Body1>
+                <b>Strategy 2021</b>
+              </Body1>
+            }
+            description={<Caption1>https://aka.ms/fluentui</Caption1>}
+            action={<Button appearance="transparent" icon={<MoreHorizontal24Regular />} />}
+          />
+        </Card>


Custom example that fits better the horizontal use case. Will start reusing more of design's templates going forward.

theerebuss · 2022-06-02T09:37:33Z

    position: 'relative',

-    '> *': {
+    [`> :not(.${cardPreviewClassNames.logo})`]: {


Previously wrongfully applying to any child element, which would include the floating logo slot.

also improves paint perf, nice catch

theerebuss · 2022-06-02T09:40:06Z

  .addDecorator(story => (
    <Screener steps={new Screener.Steps().snapshot('normal', { cropTo: '.testWrapper' }).end()}>
-      <div className="testWrapper" style={{ width: '300px' }}>
+      <div className="testWrapper" style={{ width: '600px' }}>


300px crops the horizontal example.

Hotell · 2022-06-02T11:34:54Z

+      marginTop: `calc(var(${cardCSSVars.cardSizeVar}) * -1)`,
+      marginBottom: `calc(var(${cardCSSVars.cardSizeVar}) * -1)`,
+    },
+    [`> :not([aria-hidden="true"]):first-of-type.${cardPreviewClassNames.root}`]: {


Im usually against adding comments to codebase, but I think this is the exception. can we document this pls?

Hotell · 2022-06-02T11:35:50Z

    position: 'relative',

-    '> *': {
+    [`> :not(.${cardPreviewClassNames.logo})`]: {


also improves paint perf, nice catch

msft-fluent-ui-bot assigned smhigley May 17, 2022

theerebuss unassigned smhigley May 17, 2022

theerebuss added Component: Card Fluent UI react-components (v9) labels May 17, 2022

theerebuss added this to the May Project Cycle Q2 2022 milestone May 17, 2022

theerebuss mentioned this pull request May 17, 2022

Card Convergence #19336

Closed

37 tasks

theerebuss added 9 commits May 31, 2022 00:06

feat: add orientation prop

556262c

chore: add stories

a3b1bbd

chore: update api

2499aab

chore: add vr test

0263ab9

fix: correct usage of the CSS vars

152509a

chore: add CardPreview to vr-tests

d1ae03d

chore: add better horizontal card story

83fa075

chore: add change file

6eeda03

fix: increase vr-test viewport for horizontal card

8161aab

theerebuss closed this Jun 1, 2022

theerebuss reopened this Jun 1, 2022

theerebuss modified the milestone: May Project Cycle Q2 2022 Jun 1, 2022

chore: remove CardPreview from vr-tests to reduce approval complexity

5a1b88f

github-actions Bot added this to the June Project Cycle Q2 2022 milestone Jun 2, 2022

fix: apply CardPreview styles for children only

774869c

theerebuss marked this pull request as ready for review June 2, 2022 09:39

theerebuss requested review from a team as code owners June 2, 2022 09:39

theerebuss commented Jun 2, 2022

View reviewed changes

Hotell approved these changes Jun 2, 2022

View reviewed changes

chore: add explanation of card styles

203dbb6

theerebuss requested a review from Hotell June 2, 2022 13:40

theerebuss merged commit 06030b7 into microsoft:master Jun 3, 2022

theerebuss deleted the card-orientation branch June 3, 2022 10:56

marwan38 pushed a commit to marwan38/fluentui that referenced this pull request Jun 13, 2022

feat(react-card): add orientation prop (microsoft#23037)

e47678f

Uh oh!

Conversation

theerebuss commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

New Behavior

Related Issue(s)

Uh oh!

codesandbox-ci Bot commented May 17, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fabricteam commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

size-auditor Bot commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented May 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

theerebuss Jun 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

theerebuss commented May 17, 2022 •

edited

Loading

codesandbox-ci Bot commented May 17, 2022 •

edited

Loading

fabricteam commented May 18, 2022 •

edited

Loading

size-auditor Bot commented May 18, 2022 •

edited

Loading

fabricteam commented May 18, 2022 •

edited

Loading

theerebuss Jun 3, 2022 •

edited

Loading