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.

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

feat(web-components): add tablist #32098

Merged
merged 17 commits into from
Aug 6, 2024

Conversation

davatron5000
Copy link
Contributor

Previous Behavior

Current Tabs web component is an assembly of three components (-tabs, -tab, and -tabpanel) which is very explicit but very DOM heavy and doesn't easily support scenarios like ajax'ing in content.

New Behavior

This new Tablist component is more on par with the React implementation and is much simpler in its approach, providing only the-tab and -tablist components and allowing the consumer to react to the change event (when the active tab changes) as necessary.

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 24, 2024

📊 Bundle size report

✅ No changes found

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 24, 2024

🕵 fluentui-web-components-v3 No visual regressions between this PR and main

@eljefe223
Copy link
Contributor

Should we be removing old / unused tabs and tab panel as part of this PR?

@eljefe223 eljefe223 self-requested a review July 24, 2024 16:32
@chrisdholt
Copy link
Member

General question - should our stories provide examples on how to wire up panels?

@davatron5000
Copy link
Contributor Author

davatron5000 commented Jul 24, 2024

Should we be removing old / unused tabs and tab panel as part of this PR?

@eljefe223 I talked with @chrisdholt about this yesterday and old-tabs will probably be removed/deprecated but it's probably worth doing outside of this commit since this is additive (non-breaking change) and removing tabs would potentially be a breaking change for some consumers.

General question - should our stories provide examples on how to wire up panels?

@chrisdholt I think we probably should. Storybook in its current form doesn't really allow us to show event handlers and extra script, but I'll see what I can do.

@davatron5000
Copy link
Contributor Author

RTL story and fully wired tabpanel story added to primary story in 10dc1f7.

To get these all working I needed to update the tablist class and base class...

  • $emit('change') when its done setting up the tabs
  • Run the animation loop after the tabsChanged change handler to set the correct positioning for the active animation. (Without doing this the animation would start at the far left)
  • Update the left/right arrows to be direction sensitive. This matches the React implementation.

And fwiw, the React implementation infinitely loops the roving index whereas the web component implementation stops at the end of the list. IIRC, looping back to the beginning/end is the expected behavior. This change could happen now or be an improvement later on if needed. Let me know what to do.

@chrisdholt
Copy link
Member

RTL story and fully wired tabpanel story added to primary story in 10dc1f7.

To get these all working I needed to update the tablist class and base class...

  • $emit('change') when its done setting up the tabs
  • Run the animation loop after the tabsChanged change handler to set the correct positioning for the active animation. (Without doing this the animation would start at the far left)
  • Update the left/right arrows to be direction sensitive. This matches the React implementation.

And fwiw, the React implementation infinitely loops the roving index whereas the web component implementation stops at the end of the list. IIRC, looping back to the beginning/end is the expected behavior. This change could happen now or be an improvement later on if needed. Let me know what to do.

@davatron5000 Let's look to wire up the keyboard behavior to properly loop. There's likely a couple ways this could be done. There is a utility from @microsoft/fast-web-utilities, wrapInBounds() which might make it relatively simple: https://www.npmjs.com/package/@microsoft/fast-web-utilities#wrapinbounds

packages/web-components/src/tablist/tablist.styles.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.styles.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
packages/web-components/src/tablist/tablist.ts Outdated Show resolved Hide resolved
@eljefe223 eljefe223 self-requested a review July 30, 2024 23:39
Copy link
Contributor

@eljefe223 eljefe223 left a comment

Choose a reason for hiding this comment

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

pending any open comments being resolved.

@davatron5000 davatron5000 force-pushed the feat/tablist branch 2 times, most recently from cc1d9a8 to e2a75db Compare August 1, 2024 22:30
@chrisdholt
Copy link
Member

/azp run

Copy link

Azure Pipelines successfully started running 4 pipeline(s).

@davatron5000 davatron5000 merged commit ec6b5c7 into microsoft:master Aug 6, 2024
16 of 18 checks passed
@davatron5000 davatron5000 deleted the feat/tablist branch August 6, 2024 14:01
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Aug 7, 2024
* master: (48 commits)
  chore: migrate to storybook v7 (microsoft#32018)
  release: applying package updates - react-components
  ci: make public-docsite-v9 deploy pipeline work and make it faster (microsoft#32248)
  fix(Toolbar): hardcode size of `ToolbarButton` (microsoft#32185)
  chore: deprecate @fluentui/react-motion-preview (microsoft#32190)
  release: applying package updates - web-components
  docs: add wrapping menu item example to ContextualMenu docs (microsoft#31289)
  docs: update title of v8 keyboard-accessible drag & drop example, add docs (microsoft#32216)
  release: applying package updates - react-components
  fix: Card does not override specified focusMode based on event listeners (microsoft#32200)
  Fix undefined reference in older browsers in useMessageBarReflow (microsoft#32238)
  feat: Add transparent appearance to ToolbarButton (microsoft#32205)
  fix(react-tabs): ignore ref for tab reserved space content slot (microsoft#31775)
  fix(Dialog): do not require ref forwarding (microsoft#32095)
  feat: add verify-packaging to react v8 release pipeline (microsoft#32212)
  chore(web-components): remove type-check definition and follow repo target defaults for consistency and type-check speeds (microsoft#32208)
  chore(react-tree): improve ImmutableSet and ImmutableMap internal implementation (microsoft#32167)
  release: applying package updates - web-components
  feat(web-components): add tablist (microsoft#32098)
  release: applying package updates - react-components
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants