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

[react-nav-preview]feat: Adds SplitNav*Sub*Item #32964

Open
wants to merge 22 commits into
base: master
Choose a base branch
from

Conversation

mltejera
Copy link
Contributor

@mltejera mltejera commented Oct 1, 2024

This pull request introduces the SplitNavSubItem feature to the @fluentui/react-nav-preview package. The changes focus on adding support for navSubItem within SplitNavItem components, updating the related styles, and modifying the storybook examples to demonstrate the new functionality. We also add optional Tooltips to the icon buttons.

AI Gen below:

This pull request introduces several enhancements and new features to the @fluentui/react-nav-preview package, focusing on adding support for sub-navigation items, tooltips, and other UI improvements. The changes include updates to the component props, state management, and styles to accommodate these new features.

Enhancements to Sub-Navigation and Tooltips:

Component and Style Updates:

Dependency and Documentation Updates:

Storybook Examples:

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 1, 2024

📊 Bundle size report

✅ No changes found

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 2, 2024

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

.storybook/preview.js Outdated Show resolved Hide resolved
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 2, 2024

🕵 FluentUIV0 No visual regressions between this PR and main

.storybook/preview.js Outdated Show resolved Hide resolved
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 2, 2024

Perf Analysis (@fluentui/react)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 620 612 5000
Breadcrumb mount 1703 1692 1000
Checkbox mount 1682 1718 5000
CheckboxBase mount 1494 1480 5000
ChoiceGroup mount 2894 2935 5000
ComboBox mount 693 669 1000
CommandBar mount 6516 6603 1000
ContextualMenu mount 11554 11891 1000
DefaultButton mount 794 788 5000
DetailsRow mount 2226 2202 5000
DetailsRowFast mount 2238 2242 5000
DetailsRowNoStyles mount 2065 2002 5000
Dialog mount 2739 2680 1000
DocumentCardTitle mount 233 248 1000
Dropdown mount 1968 2027 5000
FocusTrapZone mount 1153 1162 5000
FocusZone mount 1093 1109 5000
GroupedList mount 42391 42374 2
GroupedList virtual-rerender 20269 20373 2
GroupedList virtual-rerender-with-unmount 51551 51861 2
GroupedListV2 mount 220 226 2
GroupedListV2 virtual-rerender 217 216 2
GroupedListV2 virtual-rerender-with-unmount 221 229 2
IconButton mount 1141 1099 5000
Label mount 334 339 5000
Layer mount 2739 2719 5000
Link mount 388 395 5000
MenuButton mount 968 980 5000
MessageBar mount 21255 21314 5000
Nav mount 2025 2040 1000
OverflowSet mount 801 791 5000
Panel mount 1804 1796 1000
Persona mount 744 736 1000
Pivot mount 908 910 1000
PrimaryButton mount 931 941 5000
Rating mount 4751 4643 5000
SearchBox mount 928 928 5000
Shimmer mount 1887 1909 5000
Slider mount 1361 1330 5000
SpinButton mount 2949 2994 5000
Spinner mount 381 388 5000
SplitButton mount 1856 1892 5000
Stack mount 434 418 5000
StackWithIntrinsicChildren mount 868 879 5000
StackWithTextChildren mount 2734 2751 5000
SwatchColorPicker mount 6465 6462 5000
TagPicker mount 1493 1475 5000
Text mount 390 393 5000
TextField mount 918 927 5000
ThemeProvider mount 864 861 5000
ThemeProvider virtual-rerender 580 593 5000
ThemeProvider virtual-rerender-with-unmount 1309 1296 5000
Toggle mount 624 608 5000
buttonNative mount 195 194 5000

@@ -17,7 +17,7 @@ export const parameters = {
docs: {
source: {
excludeDecorators: true,
type: 'source',
type: 'code',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't check this in

Copy link
Contributor

Choose a reason for hiding this comment

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

Once this is removed we can review stuff that's owned by v-build (only this file).

@mltejera mltejera marked this pull request as ready for review October 2, 2024 20:07
@mltejera mltejera requested review from a team as code owners October 2, 2024 20:07
@mltejera mltejera force-pushed the nav/build-out-split-nav-sub-item branch from aa86d2d to 6fbc39a Compare October 3, 2024 23:05
@lekoulekou
Copy link

Dude, this is looking so good!!! Thank you so much! Visually, everything looks tip top. I'm not even tempted to open Inspect lol.

Interesting bug I found: The pin icon for SplitNavSubItem is behaving oddly. It can't reset from filled back to outline unless you close the NavCategoryItem.

Repro steps:

  • Open a NavCategoryItem (Career Development as an example)
  • Click on the pin icon for Career Paths. Icon turned blue/filled to show it's pinned.
  • Close Career Development and open it back up again.
  • The pin icon next to Career Paths will be black/filled instead of blue/filled. Clicking on the button will turn it blue/filled but will never toggle it back to the outline state unless you unpin it and close the Career Development category to reset it visually.
    image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants