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: v0 menu style migration from v9 #25012

Merged

Conversation

GianoglioEnrico
Copy link
Contributor

This PR modifies v0 Menu style to match that of v9.

We put 4px on the right and left padding on the menu to match the v9 menu style padding.
We have also added 4px border-radius to the menu item.

When hovering on a menu item the icon style change, filling with the brand color.

Current Behavior

Schermata 2022-09-29 alle 16 53 22

New Behavior

Schermata 2022-09-29 alle 16 52 09

Related Issue(s)

Fixes #

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 29, 2022

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 a7d3608:

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

@size-auditor
Copy link

size-auditor bot commented Sep 29, 2022

Asset size changes

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

Baseline commit: 7f287bc4d7b5f953d6facbf6506b147c1e2140ac (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 29, 2022

📊 Bundle size report

🤖 This report was generated against 7f287bc4d7b5f953d6facbf6506b147c1e2140ac

@fabricteam
Copy link
Collaborator

fabricteam commented Sep 29, 2022

Perf Analysis (@fluentui/react-northstar)

⚠️ 8 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
ButtonMinimalPerf.default 139 128 1.09:1 analysis
TreeWith60ListItems.default 128 120 1.07:1 analysis
RefMinimalPerf.default 189 180 1.05:1 analysis
AttachmentMinimalPerf.default 125 122 1.02:1 analysis
ChatDuplicateMessagesPerf.default 215 212 1.01:1 analysis
AvatarMinimalPerf.default 161 162 0.99:1 analysis
PortalMinimalPerf.default 147 150 0.98:1 analysis
AccordionMinimalPerf.default 113 117 0.97:1 analysis
Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
DividerMinimalPerf.default 323 309 1.05:1
ProviderMinimalPerf.default 335 319 1.05:1
StatusMinimalPerf.default 623 597 1.04:1
IconMinimalPerf.default 583 563 1.04:1
ListMinimalPerf.default 467 452 1.03:1
RadioGroupMinimalPerf.default 396 386 1.03:1
ButtonSlotsPerf.default 427 420 1.02:1
FlexMinimalPerf.default 255 251 1.02:1
InputMinimalPerf.default 841 828 1.02:1
MenuButtonMinimalPerf.default 1372 1347 1.02:1
RosterPerf.default 1718 1692 1.02:1
PopupMinimalPerf.default 561 549 1.02:1
TableMinimalPerf.default 364 357 1.02:1
ToolbarMinimalPerf.default 800 785 1.02:1
BoxMinimalPerf.default 303 300 1.01:1
CheckboxMinimalPerf.default 1547 1531 1.01:1
DropdownMinimalPerf.default 2177 2165 1.01:1
GridMinimalPerf.default 297 293 1.01:1
HeaderMinimalPerf.default 314 310 1.01:1
ImageMinimalPerf.default 346 343 1.01:1
MenuMinimalPerf.default 748 737 1.01:1
ProviderMergeThemesPerf.default 998 990 1.01:1
ReactionMinimalPerf.default 340 335 1.01:1
CustomToolbarPrototype.default 2143 2130 1.01:1
TreeMinimalPerf.default 704 697 1.01:1
VideoMinimalPerf.default 611 605 1.01:1
AlertMinimalPerf.default 224 225 1:1
AnimationMinimalPerf.default 474 472 1:1
ChatWithPopoverPerf.default 286 287 1:1
DialogMinimalPerf.default 693 692 1:1
EmbedMinimalPerf.default 2631 2637 1:1
FormMinimalPerf.default 333 332 1:1
HeaderSlotsPerf.default 679 676 1:1
LabelMinimalPerf.default 339 339 1:1
ListCommonPerf.default 519 521 1:1
SkeletonMinimalPerf.default 303 302 1:1
SplitButtonMinimalPerf.default 3282 3267 1:1
TableManyItemsPerf.default 1576 1572 1:1
TooltipMinimalPerf.default 1862 1867 1:1
ButtonOverridesMissPerf.default 1013 1019 0.99:1
ChatMinimalPerf.default 630 635 0.99:1
ItemLayoutMinimalPerf.default 964 976 0.99:1
LayoutMinimalPerf.default 314 316 0.99:1
SegmentMinimalPerf.default 301 304 0.99:1
SliderMinimalPerf.default 1227 1241 0.99:1
CardMinimalPerf.default 459 466 0.98:1
CarouselMinimalPerf.default 352 358 0.98:1
ListNestedPerf.default 462 471 0.98:1
TextMinimalPerf.default 293 300 0.98:1
TextAreaMinimalPerf.default 403 413 0.98:1
AttachmentSlotsPerf.default 856 885 0.97:1
DatepickerMinimalPerf.default 4544 4674 0.97:1
DropdownManyItemsPerf.default 529 543 0.97:1
ListWith60ListItems.default 485 498 0.97:1
LoaderMinimalPerf.default 262 512 0.51:1

@GianoglioEnrico GianoglioEnrico marked this pull request as ready for review September 29, 2022 16:30
@GianoglioEnrico GianoglioEnrico requested a review from a team as a code owner September 29, 2022 16:30
@daisygeng
Copy link
Collaborator

Could i also request that we update the divider from 2px to 1px?


@codepretty, there might be a bug in the v9 Menu
Kay originally had 'Thin' for the divider inside Menu which maps to 1px in the Tokens
image

https://www.figma.com/file/jFWrkFq61GDdOhPlsz6AtX/Menu?node-id=1528%3A5524

@daisygeng
Copy link
Collaborator

Could i also request that we update the divider from 2px to 1px?

@codepretty, there might be a bug in the v9 Menu Kay originally had 'Thin' for the divider inside Menu which maps to 1px in the Tokens image

https://www.figma.com/file/jFWrkFq61GDdOhPlsz6AtX/Menu?node-id=1528%3A5524

Opened an issue to track the v9 menu bug
#25097

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 31, 2022

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1273 1272 5000
Button mount 927 919 5000
FluentProvider mount 1493 1473 5000
FluentProviderWithTheme mount 574 587 10
FluentProviderWithTheme virtual-rerender 543 547 10
FluentProviderWithTheme virtual-rerender-with-unmount 577 579 10
MakeStyles mount 1941 1963 50000
SpinButton mount 2365 2339 5000

@jurokapsiar jurokapsiar merged commit 51eaa88 into microsoft:master Nov 1, 2022
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Nov 2, 2022
* master: (23 commits)
  fix(docsite-v9): move theme picker under component title so it can be visible on embedded pages (microsoft#25385)
  applying package updates
  feat: Implement child render function for DataGrid rows (microsoft#25476)
  fix(useTable): sort should adapt to enhanced row types (microsoft#25487)
  applying package updates
  feat: positioning should happen out of React lifecycle (microsoft#25456)
  applying package updates
  chore(react-link): migrate to new package structure (microsoft#25471)
  chore(react-input): migrate to new package structure (microsoft#25469)
  fix glob pattern for syncpack (microsoft#25465)
  chore(react-label): migrate to new package structure (microsoft#25470)
  chore: bump Griffel to latest (microsoft#25412)
  chore: add few small toolbar improvements (microsoft#25468)
  docs: fix small typos (microsoft#25464)
  chore: fix dependencies in @fluentui/react-toolbar (microsoft#25466)
  feat: v0 menu style migration from v9 (microsoft#25012)
  applying package updates
  Update List to render children on first render() call (microsoft#25331)
  Scaffold react-skeleton package (microsoft#25435)
  fix(public-docsite): Changing crossplatform urls to use 'cross' instead of 'crossplatform' (microsoft#25437)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
* Add 4px border radius when hovering on menu item

* Add 4px padding right and left to the menu

* Add subMenuIconColor to menu variables

* Add color to icons when hovering on menuItem

* Add changelog entry

* Change icon color when hovering  only for Teams theme

* Unchanged color in dark and high contrast themes

* Use a token for the icon color

* Removed check not needed

* Removed useless check

* Importing menuItemIconClassName

* Empty-Commit

Co-authored-by: Enrico <egianoglio@microsoft.com>
@khmakoto khmakoto added Fluent UI react-northstar (v0) Work related to Fluent UI V0 and removed Fluent UI react-northstar labels Nov 30, 2022
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
* Add 4px border radius when hovering on menu item

* Add 4px padding right and left to the menu

* Add subMenuIconColor to menu variables

* Add color to icons when hovering on menuItem

* Add changelog entry

* Change icon color when hovering  only for Teams theme

* Unchanged color in dark and high contrast themes

* Use a token for the icon color

* Removed check not needed

* Removed useless check

* Importing menuItemIconClassName

* Empty-Commit

Co-authored-by: Enrico <egianoglio@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants