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

fix: MenuList props should win over context props #26252

Merged
merged 2 commits into from
Jan 9, 2023

Conversation

ling1726
Copy link
Member

@ling1726 ling1726 commented Jan 9, 2023

Fixes the regression introduced in #25672 so that prop values win over context props with the same name.

Also removes deprecation notice on defaultCheckedValues and onCheckedValueChange for MenuList which are completely reasonable to use in scenarios where MenuList is used standalone.

Also updates documentation so that Menu examples will use the selection props on the root Menu components. Adds selection examples for MenuList to be more clear about the differences between the two.

Fixes #26253

Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #
@ling1726 ling1726 marked this pull request as ready for review January 9, 2023 12:11
@ling1726 ling1726 requested a review from a team as a code owner January 9, 2023 12:11
Copy link
Contributor

@bsunderhus bsunderhus left a comment

Choose a reason for hiding this comment

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

nit: are those new stories related to the fix? Shouldn't those be introduced in another PR?!

@size-auditor
Copy link

size-auditor bot commented Jan 9, 2023

Asset size changes

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

Baseline commit: 96c5a1cbb36d728bebfbf05203d372a82b1968d8 (build)

@fabricteam
Copy link
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
188.928 kB
53.088 kB
189 kB
53.116 kB
72 B
28 B
react-menu
Menu (including children components)
119.484 kB
36.932 kB
119.556 kB
36.965 kB
72 B
33 B
react-menu
Menu (including selectable components)
122.553 kB
37.452 kB
122.625 kB
37.483 kB
72 B
31 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
59.888 kB
16.652 kB
react-components
react-components: FluentProvider & webLightTheme
34.379 kB
11.322 kB
react-portal-compat
PortalCompatProvider
6.069 kB
2.053 kB
🤖 This report was generated against 96c5a1cbb36d728bebfbf05203d372a82b1968d8

@fabricteam
Copy link
Collaborator

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 1282 1284 5000
Button mount 922 925 5000
FluentProvider mount 1495 1492 5000
FluentProviderWithTheme mount 582 573 10
FluentProviderWithTheme virtual-rerender 541 544 10
FluentProviderWithTheme virtual-rerender-with-unmount 567 584 10
MakeStyles mount 1953 1936 50000
Persona mount 2832 2766 5000
SpinButton mount 2323 2320 5000

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jan 9, 2023

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 2d05db1:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration
nifty-river-og0wys Issue #26253

@fabricteam
Copy link
Collaborator

🕵 fluentuiv9 No visual regressions between this PR and main

@ling1726 ling1726 merged commit 307d798 into microsoft:master Jan 9, 2023
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Jan 10, 2023
* master: (29 commits)
  applying package updates
  fix: web component menu layering bug (microsoft#26260)
  Azure Theme: reduced spin button height to match TextField / DropDowns at 24px height (microsoft#26265)
  Update styling for contentBefore and contentAfter input slots (microsoft#26115)
  chore: Update Switch to use griffel reset styles (microsoft#26007)
  Fix: Allow root slot refs to merge with focus refs in Slider (microsoft#26243)
  applying package updates
  revert: MenuItem root slot only supports div (microsoft#26261)
  perf: Don't render Checkbox icon when unchecked (microsoft#26248)
  fix: Select disabled state hover style, Combobox disabled state open on chevron click (microsoft#26068)
  applying package updates
  chore: add more temporary codeowner rools for tooling config files (microsoft#26255)
  fix: stops using ARIAButton types for MenuItem root (microsoft#26257)
  refactor: Cleanup unused code (microsoft#26219)
  fix: MenuList props should win over context props (microsoft#26252)
  feat(react-tree):  Actions positioning and behaviour (microsoft#26113)
  BREAKING(TableCellLayout): `wrapper` slot renamed to `content` (microsoft#26220)
  fix(scripts): make lint errors reporting propagate to STDOUT during pre-commit (lint-staged exec) (microsoft#26212)
  fix: Minimum visible overflow items should be respected (microsoft#26194)
  docs: Fix typos in react-table docs (microsoft#26213)
  ...
q1b pushed a commit to q1b/fluentui that referenced this pull request Jan 24, 2023
* fix: MenuList props should win over context props

Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #

* changefile
Hotell pushed a commit to Hotell/fluentui that referenced this pull request Feb 9, 2023
* fix: MenuList props should win over context props

Fixes the regression introduced in microsoft#25672 so that prop values win over
context props with the same name.

Fixes #

* changefile
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.

MenuList props will lose to context props
5 participants