feat: Overflow menu should be registered in overflowManager by ling1726 · Pull Request #25091 · microsoft/fluentui

ling1726 · 2022-10-05T12:36:45Z

Current Behavior

The overflow manager treats the overflow menu like any other overflow item. This causes the unfortunate bug that there is no way to whether the last invisible item can simply replace the overflow menu.

New Behavior

Adds special treatment to the overflow menu so it is a special kind of item in the overflow manager.

Updates processOverflow so that there is a special case at the end to check for the case that the overflow menu can be replaced with the last overflow item.

Related Issue(s)

Fixes #25090

Fixes microsoft#25090 The overflow manager treats the overflow menu like any other overflow item. This causes the unfortunate bug that there is no way to whether the last invisible item can simply replace the overflow menu. Adds special treatment to the overflow menu so it is a special kind of item in the overflow manager. Updates `processOverflow` so that there is a special case at the end to check for the case that the overflow menu can be replaced with the last overflow item.

fabricteam · 2022-10-05T12:49:40Z

📊 Bundle size report

Package & Exports	Baseline (minified/GZIP)	PR	Change
`priority-overflow` createOverflowManager	`3.001 kB` `1.239 kB`	`3.153 kB` `1.299 kB`	`152 B` `60 B`
`react-overflow` hooks only	`10.75 kB` `4.125 kB`	`11.004 kB` `4.188 kB`	`254 B` `63 B`

Unchanged fixtures

Package & Exports	Size (minified/GZIP)
`react-components` react-components: Button, FluentProvider & webLightTheme	`62.781 kB` `17.574 kB`
`react-components` react-components: Accordion, Button, FluentProvider, Image, Menu, Popover	`189.227 kB` `52.474 kB`
`react-components` react-components: FluentProvider & webLightTheme	`33.4 kB` `11.008 kB`
`react-portal-compat` PortalCompatProvider	`5.857 kB` `1.978 kB`

_{🤖 This report was generated against d59683655d4d2a3775df4a2b41a09504cddd72ad}

size-auditor · 2022-10-05T12:51:23Z

Asset size changes

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

Baseline commit: d59683655d4d2a3775df4a2b41a09504cddd72ad (build)

fabricteam · 2022-10-05T12:51:27Z

Perf Analysis (`@fluentui/react-components`)

No significant results to display.

All results

Scenario	Render type	Master Ticks	PR Ticks	Iterations
Avatar	mount	1307	1293	5000
Button	mount	951	946	5000
FluentProvider	mount	1569	1581	5000
FluentProviderWithTheme	mount	635	642	10
FluentProviderWithTheme	virtual-rerender	595	578	10
FluentProviderWithTheme	virtual-rerender-with-unmount	617	619	10
MakeStyles	mount	1880	1900	50000
SpinButton	mount	2538	2517	5000

codesandbox-ci · 2022-10-05T12:51:47Z

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

Sandbox	Source
@fluentui/react 8 starter	Configuration
@fluentui/react-components 9 starter	Configuration
peaceful-wiles-x77ini	Issue #25090

ling1726 · 2022-10-05T13:26:50Z

packages/react-components/priority-overflow/src/priorityQueue.ts

    const i = arr.indexOf(item);

-    if (i === -1) {
+    if (i === -1 || i >= size) {


these were absolutely awful bugs

packages/react-components/react-overflow/src/useOverflowMenu.ts

packages/react-components/priority-overflow/src/priorityQueue.ts

packages/react-components/priority-overflow/src/overflowManager.ts

layershifter

Are you going to add some tests?

ling1726 · 2022-10-07T15:40:25Z

Are you going to add some tests?

Added it, I was waiting for #25122 to setup the tests

* master: (23 commits) Revert "chore: screener-run workflow should report to PR (microsoft#25144)" (microsoft#25145) chore: screener-run workflow should report to PR (microsoft#25144) applying package updates fix: The Tooltip should not hide if it gets keyboard focus (microsoft#25138) fix: Tooltip should not hide if an element inside it gets focused (microsoft#25140) Create react-migration-v8-v9 with shims and stories (microsoft#25121) fix: CheckboxField to set a generated ID on the input, to match the label's htmlFor (microsoft#25079) feat: Overflow menu should be registered in overflowManager (microsoft#25091) fix: version-bump generator removes beachball disallowedChangeType config (microsoft#25130) fix: new overflow items should only be enqueued while observing (microsoft#25122) fix(script): allow runPublished call from CLI (microsoft#25127) feat: add vertical variation for toolbar (microsoft#24940) ProgressField implementation and stories (microsoft#25103) fix: Dropdown icon layout with no placeholder/value (microsoft#25049) chore: add a bundle size fixture (Button, Provider & theme) (microsoft#25113) feat: Adding subtle transition between states on Button components (microsoft#25106) Revert "chore: screener-run workflow should report to PR (microsoft#25096)" (microsoft#25115) chore: screener-run workflow should report to PR (microsoft#25096) fix(react-dialog): aria-* properties should be reassignable (microsoft#25092) fix(scripts): don't run publish if web-components are affected (microsoft#25095) ...

…t#25091) * feat: Overflow menu should be registered in overflowManager Fixes microsoft#25090 The overflow manager treats the overflow menu like any other overflow item. This causes the unfortunate bug that there is no way to whether the last invisible item can simply replace the overflow menu. Adds special treatment to the overflow menu so it is a special kind of item in the overflow manager. Updates `processOverflow` so that there is a special case at the end to check for the case that the overflow menu can be replaced with the last overflow item. * changefile * update md * add test * revert * remove cruft * rename overflowMenuSize to overflowMenuOffset

This fixes a regression from microsoft#25091, the updated mechanism to handle overflow menu was inadvertedly breaking the minumum visible options because it was still making items invisible without considering whether the minimum number of items was reached.

* fix: Minimum visible overflow items should be respected This fixes a regression from #25091, the updated mechanism to handle overflow menu was inadvertedly breaking the minumum visible options because it was still making items invisible without considering whether the minimum number of items was reached. * changefile * add test * changefile

) * fix: Minimum visible overflow items should be respected This fixes a regression from microsoft#25091, the updated mechanism to handle overflow menu was inadvertedly breaking the minumum visible options because it was still making items invisible without considering whether the minimum number of items was reached. * changefile * add test * changefile

msft-fluent-ui-bot assigned miroslavstastny Oct 5, 2022

github-actions bot added this to the October Project Cycle Q4 2022 milestone Oct 5, 2022

ling1726 added 2 commits October 5, 2022 14:38

changefile

9186714

update md

66febd4

ling1726 commented Oct 5, 2022

View reviewed changes

layershifter reviewed Oct 5, 2022

View reviewed changes

packages/react-components/react-overflow/src/useOverflowMenu.ts Outdated Show resolved Hide resolved

layershifter reviewed Oct 5, 2022

View reviewed changes

packages/react-components/priority-overflow/src/priorityQueue.ts Show resolved Hide resolved

layershifter reviewed Oct 5, 2022

View reviewed changes

packages/react-components/priority-overflow/src/overflowManager.ts Outdated Show resolved Hide resolved

layershifter reviewed Oct 5, 2022

View reviewed changes

ling1726 added 2 commits October 7, 2022 17:26

merge master

ddbae87

add test

52b9c0d

ling1726 added 2 commits October 7, 2022 17:42

revert

123c955

remove cruft

7ecdcb7

ling1726 marked this pull request as ready for review October 7, 2022 15:44

ling1726 requested a review from a team as a code owner October 7, 2022 15:44

rename overflowMenuSize to overflowMenuOffset

c2a1a45

ling1726 requested a review from layershifter October 7, 2022 15:45

flora8984461 mentioned this pull request Oct 7, 2022

[Bug]: Overflow does not work properly with tablist #23555

Closed

2 tasks

layershifter approved these changes Oct 7, 2022

View reviewed changes

ling1726 merged commit 6440417 into microsoft:master Oct 7, 2022

ling1726 mentioned this pull request Jan 6, 2023

fix: Minimum visible overflow items should be respected #26194

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Overflow menu should be registered in overflowManager#25091

feat: Overflow menu should be registered in overflowManager#25091
ling1726 merged 8 commits intomicrosoft:masterfrom
ling1726:feat/register-overflow-menu

ling1726 commented Oct 5, 2022

Uh oh!

fabricteam commented Oct 5, 2022 •

edited

Loading

Uh oh!

size-auditor bot commented Oct 5, 2022 •

edited

Loading

Uh oh!

fabricteam commented Oct 5, 2022 •

edited

Loading

Uh oh!

codesandbox-ci bot commented Oct 5, 2022 •

edited

Loading

Uh oh!

ling1726 Oct 5, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layershifter left a comment

Uh oh!

ling1726 commented Oct 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ling1726 commented Oct 5, 2022

Current Behavior

New Behavior

Related Issue(s)

Uh oh!

fabricteam commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Bundle size report

Uh oh!

size-auditor bot commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Asset size changes

Uh oh!

fabricteam commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf Analysis (@fluentui/react-components)

Uh oh!

codesandbox-ci bot commented Oct 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ling1726 Oct 5, 2022

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

layershifter left a comment

Choose a reason for hiding this comment

Uh oh!

ling1726 commented Oct 7, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

fabricteam commented Oct 5, 2022 •

edited

Loading

size-auditor bot commented Oct 5, 2022 •

edited

Loading

fabricteam commented Oct 5, 2022 •

edited

Loading

Perf Analysis (`@fluentui/react-components`)

codesandbox-ci bot commented Oct 5, 2022 •

edited

Loading