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

Add mountNode prop to combos #28661

Merged
merged 5 commits into from
Jul 28, 2023

Conversation

GeoffCoxMSFT
Copy link
Member

@GeoffCoxMSFT GeoffCoxMSFT commented Jul 27, 2023

Issue

Allows callers to change the mountNode of the portal used to display the dropdown part for ComboBox and Dropdown.

Fixes #28625

Changes

  • Added mountNode to ComboboxBaseProps
  • Updated Combobox and Dropdown to pass mountNode to Portal.

I created a storybook example to demonstrate it working. I don't think this makes a good story to check in for combobox though.

Combobox.mount.node.mov

@GeoffCoxMSFT GeoffCoxMSFT self-assigned this Jul 27, 2023
@GeoffCoxMSFT GeoffCoxMSFT requested review from a team and smhigley as code owners July 27, 2023 18:22
@github-actions github-actions bot added this to the July Project Cycle Q3 2023 milestone Jul 27, 2023
@fabricteam
Copy link
Collaborator

fabricteam commented Jul 27, 2023

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 599 604 5000
Button mount 291 289 5000
Field mount 1061 1043 5000
FluentProvider mount 630 654 5000
FluentProviderWithTheme mount 76 78 10
FluentProviderWithTheme virtual-rerender 67 66 10
FluentProviderWithTheme virtual-rerender-with-unmount 66 76 10
InfoButton mount 11 13 5000
MakeStyles mount 860 847 50000
Persona mount 1667 1578 5000
SpinButton mount 1367 1327 5000

@codesandbox-ci
Copy link

codesandbox-ci bot commented Jul 27, 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 c0726d2:

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

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 27, 2023

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-combobox
Combobox (including child components)
87.915 kB
28.342 kB
87.958 kB
28.359 kB
43 B
17 B
react-combobox
Dropdown (including child components)
86.339 kB
27.964 kB
86.382 kB
27.983 kB
43 B
19 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
67.576 kB
18.225 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
208.062 kB
57.968 kB
react-components
react-components: FluentProvider & webLightTheme
36.409 kB
12.003 kB
react-portal-compat
PortalCompatProvider
6.48 kB
2.203 kB
🤖 This report was generated against bdbf3ae799e368080db7af64f385f5c35c4c8fdf

@fabricteam
Copy link
Collaborator

fabricteam commented Jul 27, 2023

🕵 fluentuiv9 No visual regressions between this PR and main

@size-auditor
Copy link

size-auditor bot commented Jul 27, 2023

Asset size changes

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

Baseline commit: bdbf3ae799e368080db7af64f385f5c35c4c8fdf (build)

Copy link
Contributor

@smhigley smhigley left a comment

Choose a reason for hiding this comment

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

Thanks so much for making this!

@layershifter
Copy link
Member

Can we be consistent with other components and expose mountNode prop instead?



export type PopoverProps = Pick<PortalProps, 'mountNode'> & {

@GeoffCoxMSFT
Copy link
Member Author

Can we be consistent with other components and expose mountNode prop instead?

export type PopoverProps = Pick<PortalProps, 'mountNode'> & {

That was my original plan, then others suggested making it a slot. LOL. I will update to follow the precedent.

@GeoffCoxMSFT GeoffCoxMSFT changed the title Add portal slot to combos Add mountNode prop to combos Jul 28, 2023
@GeoffCoxMSFT GeoffCoxMSFT enabled auto-merge (squash) July 28, 2023 20:24
@GeoffCoxMSFT GeoffCoxMSFT merged commit 50392ee into microsoft:master Jul 28, 2023
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Aug 1, 2023
* master: (37 commits)
  release (microsoft#28696)
  Fixing re-render issue for all charts when empty (microsoft#28321)
  feat(FluentProvider): emit errors on duplicate IDs (microsoft#28670)
  applying package updates
  fix(react-positioning): autoSize causing position update to reach maximum (microsoft#28689)
  fix(react-tags-preview): fix InteractionTag hover styles (microsoft#28686)
  Accordion: export AccordionHeaderProvider (microsoft#28542)
  feat(react-shared-contexts): add AnnounceContext (microsoft#28654)
  Added VR tests for Breadcrumb (microsoft#28653)
  fix(react-menu): use outline for menuItem focus ring (microsoft#28685)
  [Bug]: Tree, vertical spacing of branches and children is inconsistent (microsoft#28681)
  feaTt(react-tree): adds openItems and checkedItems to tree callback data (microsoft#28669)
  applying package updates
  Add mountNode prop to combos (microsoft#28661)
  react-tags-preview: add more vr test (microsoft#28582)
  chore: migrate to nx 16.1.4 (microsoft#28583)
  applying package updates
  chore: improves internal headless signature (microsoft#28651)
  fix: remove margin from icon when ToolbarButton is vertical (microsoft#28658)
  applying package updates
  ...
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.

[Feature]: Add mountNode property to Combobox
4 participants