Card: Improving styling and adding compact Card styling and examples #8247

khmakoto · 2019-03-08T22:14:55Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ npm run change

Description of changes

This PR improves styling in the Card component by aligning it closer to the examples shown by design. It also adds styling for compact Cards and adds an example of it to the component page. Finally, this PR also adds a basic and a compact Card with no content in it aside from text to show that the component refers to the container and not its inner components.

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

…kage.

…c-react

…c-react into cardApi

size-auditor · 2019-03-08T22:35:03Z

Bundle test	Size (minified)	Diff from master
Stack	42.265 kB	69 bytes
Foundation	49.857 kB	18 bytes
Text	39.131 kB	18 bytes

Exceeds Tolerance Exceeds Baseline Below Baseline 1 kB = 1000 bytes

packages/react-cards/src/components/Card/CardItem/CardItem.types.ts

dzearing · 2019-03-13T01:34:23Z

packages/react-cards/src/components/Card/examples/Card.Compact.Example.tsx

+        </Card>
+
+        <Card compact={true} gap={12} tokens={{ padding: 12 }} onClick={alertClicked}>
+          <Card.Item tokens={{ margin: '-12px 0 -12px -12px' }}>


Not for change now, but I would like to see no hardcoded gap sizes or paddings in any of the docs.

Allowing or encouraging hardcoded sizes like this in the code I think will have some bad consequences. People will have opinions which will create inconsistency.

It also makes the implementation messy.

I removed both gap and padding hardcoded sizes for the Card usage in the examples. Sadly, I didn't remove the margins in Card.Item because it would break some inner styling right now.

…c-react into cardApi

packages/office-ui-fabric-react/src/components/Stack/StackItem/StackItem.styles.ts

JasonGore · 2019-03-13T20:03:54Z

packages/react-cards/src/components/Card/Card.types.ts

+  /**
+   * Defines the spacing between Card children.
+   */
+  gap?: number | string;


David and I have talked previously about how this is something that should be accessible via theming. This is fairly awkward to have to specify as a standalone prop in every component instance. I wonder if gap is actually a token idea that needs a better name. Maybe contentPadding? Hmmmmmmm......

Should we consider this as part of this PR or does this need further discussion? In any case, if we actually change gap to something else we will have to consider that changing gap in Stack is a breaking change.

Is react-cards still in experimental state? If not, I'd rather consider this as part of this PR rather than set gap in stone for Card.

Regarding Stack, I don't think it would have to be a breaking change. Could we not support both and deprecate gap?

We can do it in this PR, I was just asking. And yeah, I suppose we can deprecate gap and support the other approach.

Do you want contentPadding to be the token name? Or should we do more brainstorming on the name?

…c-react into cardApi

leddie24 · 2019-03-14T22:11:50Z

@kkjeer , looks like this still needs your approval as a code owner; can you take a look at your earliest convenience please?

leddie24 · 2019-03-14T23:15:55Z

Just pinged @khmakoto on this, he's still adding changes to the PR, but will merge when ready.

JasonGore · 2019-03-15T01:03:41Z

packages/office-ui-fabric-react/src/components/Stack/Stack.types.ts

@@ -84,6 +84,7 @@ export interface IStackProps
   * Defines the spacing between Stack children.
   * The property is specified as a value for 'row gap', followed optionally by a value for 'column gap'.
   * If 'column gap' is omitted, it's set to the same value as 'row gap'.
+   * @deprecated Use `childrenGap` token instead.


We should add a warnDeprecations call for this too. I'm not sure how difficult that is since we're not using BaseComponent, but maybe there's an underlying utility function we can call directly...

Perhaps https://github.com/OfficeDev/office-ui-fabric-react/blob/master/packages/utilities/src/warn/warnDeprecations.ts?

…c-react into cardApi

JasonGore · 2019-03-29T14:39:53Z

...es/office-ui-fabric-react/src/components/Stack/examples/Stack.Horizontal.Spacing.Example.tsx

        <Stack horizontal disableShrink horizontalAlign="space-between">
          <Stack>
            <span>Numerical spacing</span>
-            <Stack horizontal className={styles.root} gap={10} padding={10}>
+            <Stack horizontal className={styles.root} tokens={tokens.numericalSpacing} padding={10}>


There's still a lot of duplication going on. Once we get extendsComponent in we can do something like:

const NumberStack = extendComponent(Stack, { horizontal: true, styles: { }, padding: 10, tokens: { childrenGap: 10 } });

Then just use:

<NumberStack> 1 2 3 </NumberStack>

JasonGore

We can improve this further and remove duplication once we have extendComponent.

ecraig12345 · 2019-03-29T17:48:33Z

packages/react-cards/webpack.serve.config.js

@@ -18,9 +15,17 @@ module.exports = resources.createServeConfig({

  resolve: {
    alias: {
+      '@uifabric/example-app-base$': path.join(__dirname, '../../packages/example-app-base/src'),


These should probably use path.resolve not path.join.

msft-github-bot · 2019-04-01T12:39:38Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-04-01T12:39:38Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-04-01T12:39:47Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-04-01T12:39:48Z

🎉@uifabric/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

msft-github-bot · 2019-04-01T12:39:55Z

🎉[email protected] has been released which incorporates this pull request.:tada:

Handy links:

khmakoto added 6 commits March 6, 2019 16:03

Adding pre-copy.json creation step and template when using create-pac…

15eacf1

…kage.

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

f652242

…c-react

Improving styling of basic Cards and adding example of compact Card.

fa82413

Adding compact Card styling and examples.

b544d3c

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

5ae2539

…c-react into cardApi

Adding change file.

a3e7d7e

khmakoto added the Component: Card label Mar 8, 2019

khmakoto requested review from jdhuntington, Jahnp, dzearing, micahgodbolt and JasonGore March 8, 2019 22:14

msft-github-bot assigned leddie24 Mar 8, 2019

JasonGore reviewed Mar 12, 2019

View reviewed changes

packages/react-cards/src/components/Card/CardItem/CardItem.types.ts Outdated Show resolved Hide resolved

JasonGore reviewed Mar 12, 2019

View reviewed changes

packages/react-cards/src/components/Card/CardItem/CardItem.types.ts Outdated Show resolved Hide resolved

dzearing reviewed Mar 13, 2019

View reviewed changes

Humberto Makoto Morimoto Burgos added 2 commits March 13, 2019 08:49

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

6d57ee1

…c-react into cardApi

Adding margin as token to StackItem and removing CardItem types.

c0ae063

khmakoto requested a review from kkjeer as a code owner March 13, 2019 19:45

Removing gaps and paddings from examples

499aade

JasonGore reviewed Mar 13, 2019

View reviewed changes

Humberto Makoto Morimoto Burgos added 3 commits March 13, 2019 15:50

Leaving margin token as undefined.

85e4866

Adding OUFR change file.

752a7ad

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

d92c479

…c-react into cardApi

kkjeer approved these changes Mar 14, 2019

View reviewed changes

Adding childrenGap token to Stack and Card.

5969ffc

JasonGore reviewed Mar 15, 2019

View reviewed changes

JasonGore mentioned this pull request Mar 28, 2019

FocusZone and FocusTrapZone example improvements #8505

Merged

1 task

khmakoto added 5 commits March 28, 2019 11:08

Updating examples to centralize tokens instead of using them inline.

9b2290b

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

bb7bc1e

…c-react into cardApi

Updating Dropdown examples to have token centralization.

834b48d

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

db83ae5

…c-react into cardApi

Adding merge-styles change file.

0d1e5b2

khmakoto requested a review from JasonGore March 28, 2019 20:36

khmakoto added 2 commits March 28, 2019 16:05

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

90a58d5

…c-react into cardApi

Updating TextField examples.

3256fc1

JasonGore reviewed Mar 29, 2019

View reviewed changes

Updating Spinner and Slider examples.

93442a2

khmakoto requested review from aneeshack4 and joschect as code owners March 29, 2019 16:51

JasonGore approved these changes Mar 29, 2019

View reviewed changes

Removing deprecation detection for Stack.

249fade

ecraig12345 reviewed Mar 29, 2019

View reviewed changes

Using path.resolve instead of path.join in webpack.serve.config.js.

4434317

ecraig12345 approved these changes Mar 29, 2019

View reviewed changes

natalieethell approved these changes Mar 29, 2019

View reviewed changes

joschect approved these changes Mar 29, 2019

View reviewed changes

JasonGore merged commit df35977 into microsoft:master Mar 29, 2019

khmakoto deleted the cardApi branch March 29, 2019 21:28

khmakoto mentioned this pull request Apr 2, 2019

Button: Fixing SplitButton styling issues and correctly placing divider #8561

Merged

2 tasks

micahgodbolt mentioned this pull request Apr 2, 2019

New Component: Card #5322

Closed

12 tasks

microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Card: Improving styling and adding compact Card styling and examples #8247

Card: Improving styling and adding compact Card styling and examples #8247

khmakoto commented Mar 8, 2019 •

edited by msft-github-bot

Loading

size-auditor bot commented Mar 8, 2019 •

edited

Loading

dzearing Mar 13, 2019 •

edited

Loading

khmakoto Mar 13, 2019

JasonGore Mar 13, 2019

khmakoto Mar 13, 2019

JasonGore Mar 13, 2019

khmakoto Mar 13, 2019

leddie24 commented Mar 14, 2019

leddie24 commented Mar 14, 2019

JasonGore Mar 15, 2019

KevinTCoughlin Mar 15, 2019

JasonGore Mar 29, 2019

JasonGore left a comment

ecraig12345 Mar 29, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

Card: Improving styling and adding compact Card styling and examples #8247

Card: Improving styling and adding compact Card styling and examples #8247

Conversation

khmakoto commented Mar 8, 2019 • edited by msft-github-bot Loading

Pull request checklist

Description of changes

Focus areas to test

Microsoft Reviewers: Open in CodeFlow

size-auditor bot commented Mar 8, 2019 • edited Loading

dzearing Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leddie24 commented Mar 14, 2019

leddie24 commented Mar 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JasonGore left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

msft-github-bot commented Apr 1, 2019

khmakoto commented Mar 8, 2019 •

edited by msft-github-bot

Loading

size-auditor bot commented Mar 8, 2019 •

edited

Loading

dzearing Mar 13, 2019 •

edited

Loading