Skip to content

feat(react-drawer): dialog slot to be only used by internal composition API#29279

Closed
marcosmoura wants to merge 2 commits into
microsoft:masterfrom
marcosmoura:feat/react-drawer/make-dialog-slot-internal-to-composition
Closed

feat(react-drawer): dialog slot to be only used by internal composition API#29279
marcosmoura wants to merge 2 commits into
microsoft:masterfrom
marcosmoura:feat/react-drawer/make-dialog-slot-internal-to-composition

Conversation

@marcosmoura

Copy link
Copy Markdown
Contributor

Previous Behavior

Dialog was created as an external slot and users could easily override to any other component with the same signature. That could lead to a completely broken Drawer as it is heavily dependent on Dialog.

New Behavior

Makes the Dialog as an internal slot. That makes it possible to override the Dialog component only when explicitly using the composition API.

@marcosmoura
marcosmoura requested a review from a team as a code owner September 25, 2023 13:56
@marcosmoura marcosmoura self-assigned this Sep 25, 2023
@marcosmoura marcosmoura added the Component: Drawer The Fluent v9 Drawer component label Sep 25, 2023
@marcosmoura marcosmoura changed the title feat(react-drawer): make dialog slot to be used for composition only feat(react-drawer): dialog slot to be only used by internal composition API Sep 25, 2023
@codesandbox-ci

Copy link
Copy Markdown

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 322eb4b:

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

@fabricteam

Copy link
Copy Markdown
Collaborator

Perf Analysis (@fluentui/react-components)

Scenario Render type Master Ticks PR Ticks Iterations Status
InfoButton mount 15 14 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 615 655 5000
Button mount 302 294 5000
Field mount 1102 1124 5000
FluentProvider mount 696 691 5000
FluentProviderWithTheme mount 79 85 10
FluentProviderWithTheme virtual-rerender 68 69 10
FluentProviderWithTheme virtual-rerender-with-unmount 74 75 10
InfoButton mount 15 14 5000 Possible regression
MakeStyles mount 860 865 50000
Persona mount 1723 1709 5000
SpinButton mount 1405 1378 5000

@fabricteam

Copy link
Copy Markdown
Collaborator

📊 Bundle size report

Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
69.572 kB
19.658 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
208.688 kB
59.552 kB
react-components
react-components: FluentProvider & webLightTheme
40.966 kB
13.569 kB
react-portal-compat
PortalCompatProvider
6.541 kB
2.227 kB
🤖 This report was generated against c1ae0fb42575ae2deb11b32110538adbd0f652ad

@size-auditor

size-auditor Bot commented Sep 25, 2023

Copy link
Copy Markdown

Asset size changes

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

Baseline commit: c1ae0fb42575ae2deb11b32110538adbd0f652ad (build)

"comment": "feat: make dialog slot internal to be used for composition only",
"packageName": "@fluentui/react-drawer",
"email": "marcosvmmoura@gmail.com",
"dependentChangeType": "patch"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Don't forget to update in to feature

@marcosmoura

Copy link
Copy Markdown
Contributor Author

This PR is not needed as #29392 now have all of these changes.

@marcosmoura marcosmoura closed this Oct 5, 2023
@marcosmoura
marcosmoura deleted the feat/react-drawer/make-dialog-slot-internal-to-composition branch November 15, 2023 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: Drawer The Fluent v9 Drawer component

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants