Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs(react-skeleton): extend Skeleton story with SkeletonItem examples #31608

Merged
merged 1 commit into from
Jun 11, 2024
Merged

docs(react-skeleton): extend Skeleton story with SkeletonItem examples #31608

merged 1 commit into from
Jun 11, 2024

Conversation

mainframev
Copy link
Contributor

@mainframev mainframev commented Jun 6, 2024 •
edited
Loading

Resolves #31222. Extended Skeleton story to have SkeletonItem examples for size and shape props. Tbh, not sure if it deserves completely separated tab, as SkeletonItem isn't standalone and supposed to be used as a child component of Skeleton.

Related Issue(s)

#31222

@mainframev mainframev force-pushed the docs/extend-skeleton-stories branch from a50dc14 to 2b989ac Compare June 6, 2024 17:20
@fabricteam
Copy link
Collaborator

fabricteam commented Jun 6, 2024 •
edited
Loading

📊 Bundle size report

✅ No changes found

@fabricteam
Copy link
Collaborator

fabricteam commented Jun 6, 2024 •
edited
Loading

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
FluentProviderWithTheme virtual-rerender 36 39 10 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 617 618 5000
Button mount 302 320 5000
Field mount 1145 1141 5000
FluentProvider mount 711 713 5000
FluentProviderWithTheme mount 80 76 10
FluentProviderWithTheme virtual-rerender 36 39 10 Possible regression
FluentProviderWithTheme virtual-rerender-with-unmount 73 76 10
MakeStyles mount 868 858 50000
Persona mount 1715 1747 5000
SpinButton mount 1354 1404 5000
SwatchPicker mount 1500 1515 5000

@mainframev mainframev marked this pull request as ready for review June 6, 2024 22:04
@mainframev mainframev requested a review from a team as a code owner June 6, 2024 22:04
@mainframev mainframev requested a review from a team June 6, 2024 22:04
@mainframev mainframev self-assigned this Jun 6, 2024
@mainframev mainframev force-pushed the docs/extend-skeleton-stories branch from c28c05c to 37ff9e1 Compare June 10, 2024 09:13
@ValentinaKozlova
Copy link
Contributor

I see that in many examples SkeletonItem gets size prop. It's interesting if it's a common usage to have many items. If it is, probably it would be good to pass size from the parent component Skeleton. It doesn't refer to this PR, just an idea :)

@mainframev
Copy link
Contributor Author

mainframev commented Jun 10, 2024 •
edited
Loading

I see that in many examples SkeletonItem gets size prop. It's interesting if it's a common usage to have many items. If it is, probably it would be good to pass size from the parent component Skeleton. It doesn't refer to this PR, just an idea :)

Yeah, makes sense to me. Current API is that they share two same props, SkeletonItem has appearance and animation as well as main Skeleton :) So following this logic, size prop can be added to Skeleton as well :)

@spmonahan
Copy link
Contributor

I see that in many examples SkeletonItem gets size prop. It's interesting if it's a common usage to have many items. If it is, probably it would be good to pass size from the parent component Skeleton. It doesn't refer to this PR, just an idea :)

Make a feature issue please!

smhigley
smhigley approved these changes Jun 10, 2024
View reviewed changes
Copy link
Contributor

@smhigley smhigley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small naming nit/suggestion, otherwise looks good!

@mainframev mainframev force-pushed the docs/extend-skeleton-stories branch from 43b0f3f to 43e9138 Compare June 11, 2024 08:07
@mainframev
docs(react-skeleton): extend Skeleton story with SkeletonItem examples
5b7a176 
docs(react-skeleton): extend Skeleton story with SkeletonItem examples

docs(react-skeleton): extend Skeleton story with SkeletonItem examples

docs(react-skeleton): extend Skeleton story with SkeletonItem examples
@mainframev mainframev force-pushed the docs/extend-skeleton-stories branch from 43e9138 to 5b7a176 Compare June 11, 2024 08:07
@mainframev mainframev merged commit a839e63 into microsoft:master Jun 11, 2024
19 checks passed
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 11, 2024
@marcosmoura
Merge branch 'master' into feat/react-nav-preview/transition-examples…
4ec1cb8 
…-and-drawer-compat

* master: (43 commits)
  chore: remove react-alert from monorepo (microsoft#31642)
  docs(react-skeleton): extend Skeleton story with SkeletonItem examples (microsoft#31608)
  feat(react-motion): add support for params (microsoft#31566)
  applying package updates
  fix: show default title action in dialog body for modal dialogs (microsoft#31648)
  chore:(react-nav-preview)Remove redundant NavDrawerHeaderNav component. (microsoft#31646)
  Update Accordion Size story to allow collapsing (microsoft#31624)
  fix(react-accordion): deprecate navigation prop (microsoft#31587)
  fix: Drawer story accessibility fixes and docs update (microsoft#31570)
  feat:(react-nav-preview) Adds small size variant (microsoft#31589)
  feat: update divider to use element internals (microsoft#31627)
  chore(react-components): split react libraries in two (/library and /stories) - teams-prg /3rd batch (microsoft#31601)
  chore:(docs) Adding Jest testing document (microsoft#31554)
  chore(react-components): split react libraries in two (/library and /stories) - teams-prg /2nd batch (microsoft#31600)
  build(deps): bump tar from 6.1.11 to 6.2.1 (microsoft#31633)
  applying package updates
  fix: allow updating of CSS properties when they are already defined (microsoft#31629)
  fix: corrects the border-color for switch when in the checked state on rest (microsoft#31628)
  chore: update Switch to leverage ElementInternals via Checkbox (microsoft#31613)
  chore: update temporarily codeowners for split-in-two migrated packages to maintain proper PR review assignemnt for outdated branches (microsoft#31616)
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jun 11, 2024
@marcosmoura
Merge branch 'master' into rfc/z-index-handling
8cf78e9 
* master: (43 commits)
  chore: remove react-alert from monorepo (microsoft#31642)
  docs(react-skeleton): extend Skeleton story with SkeletonItem examples (microsoft#31608)
  feat(react-motion): add support for params (microsoft#31566)
  applying package updates
  fix: show default title action in dialog body for modal dialogs (microsoft#31648)
  chore:(react-nav-preview)Remove redundant NavDrawerHeaderNav component. (microsoft#31646)
  Update Accordion Size story to allow collapsing (microsoft#31624)
  fix(react-accordion): deprecate navigation prop (microsoft#31587)
  fix: Drawer story accessibility fixes and docs update (microsoft#31570)
  feat:(react-nav-preview) Adds small size variant (microsoft#31589)
  feat: update divider to use element internals (microsoft#31627)
  chore(react-components): split react libraries in two (/library and /stories) - teams-prg /3rd batch (microsoft#31601)
  chore:(docs) Adding Jest testing document (microsoft#31554)
  chore(react-components): split react libraries in two (/library and /stories) - teams-prg /2nd batch (microsoft#31600)
  build(deps): bump tar from 6.1.11 to 6.2.1 (microsoft#31633)
  applying package updates
  fix: allow updating of CSS properties when they are already defined (microsoft#31629)
  fix: corrects the border-color for switch when in the checked state on rest (microsoft#31628)
  chore: update Switch to leverage ElementInternals via Checkbox (microsoft#31613)
  chore: update temporarily codeowners for split-in-two migrated packages to maintain proper PR review assignemnt for outdated branches (microsoft#31616)
  ...
@ValentinaKozlova ValentinaKozlova mentioned this pull request Jun 11, 2024
Open
1 task
@ValentinaKozlova
Copy link
Contributor

I see that in many examples SkeletonItem gets size prop. It's interesting if it's a common usage to have many items. If it is, probably it would be good to pass size from the parent component Skeleton. It doesn't refer to this PR, just an idea :)

Make a feature issue please!

done #31664 :)

miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
@mainframev @miroslavstastny
docs(react-skeleton): extend Skeleton story with SkeletonItem examples (
7b49267 
microsoft#31608)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smhigley smhigley smhigley approved these changes

@fabricteam fabricteam fabricteam left review comments

@marcosmoura marcosmoura marcosmoura approved these changes

@khmakoto khmakoto khmakoto approved these changes

Assignees

@mainframev mainframev

Labels
Component: Skeleton Fluent UI react-components (v9) Storybook
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Bug]: SkeletonItem does not have a story in the storybook
7 participants
@mainframev @fabricteam @ValentinaKozlova @spmonahan @marcosmoura @smhigley @khmakoto