Skip to content

Conversation

@bsunderhus
Copy link
Contributor

@bsunderhus bsunderhus commented Oct 4, 2023

Previous Behavior

useDialogSurfaceStyles is invoking useDialogContext internally, according to our composition API each composition hook should be independant, and any dependency should be provided through arguments (in the style case, through state)

New Behavior

  1. fixes DialogSurfaceState to include necessary value to make useDialogSurfaceStyles independent from useDialogContext hook
  2. removes native dialog styles (as we don't support native dialog anymore)

Fixes

Problem originally identified here #29392 (comment)

@github-actions github-actions bot added this to the October Project Cycle Q4 2023 milestone Oct 4, 2023
@bsunderhus bsunderhus force-pushed the react-dialog/bugfix/removes-context-hook-from-styles-hook branch from dc64b9e to 760e518 Compare October 4, 2023 17:32
@bsunderhus bsunderhus self-assigned this Oct 4, 2023
@bsunderhus bsunderhus marked this pull request as ready for review October 4, 2023 17:35
@bsunderhus bsunderhus requested a review from a team as a code owner October 4, 2023 17:35
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 4, 2023

Perf Analysis (@fluentui/react-components)

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 614 622 5000
Button mount 323 311 5000
Field mount 1120 1103 5000
FluentProvider mount 686 690 5000
FluentProviderWithTheme mount 81 75 10
FluentProviderWithTheme virtual-rerender 61 65 10
FluentProviderWithTheme virtual-rerender-with-unmount 74 68 10
InfoButton mount 15 8 5000
MakeStyles mount 848 835 50000
Persona mount 1687 1692 5000
SpinButton mount 1335 1362 5000

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 4, 2023

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-dialog
Dialog (including children components)
88.365 kB
26.344 kB
88.302 kB
26.325 kB
-63 B
-19 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
68.748 kB
19.742 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
206.475 kB
59.3 kB
react-components
react-components: FluentProvider & webLightTheme
40.866 kB
13.546 kB
react-portal-compat
PortalCompatProvider
6.503 kB
2.22 kB
🤖 This report was generated against b4611f7230fa90d1f01ab92ded39ebe2f02298f3

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 4, 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 70cf9a4:

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

@size-auditor
Copy link

size-auditor bot commented Oct 4, 2023

Asset size changes

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

Baseline commit: b4611f7230fa90d1f01ab92ded39ebe2f02298f3 (build)

+ removes native backdrop styles (as we don't support native dialog)
@bsunderhus bsunderhus force-pushed the react-dialog/bugfix/removes-context-hook-from-styles-hook branch from 760e518 to 70cf9a4 Compare October 4, 2023 17:54
@fabricteam
Copy link
Collaborator

fabricteam commented Oct 4, 2023

🕵 fluentuiv9 No visual regressions between this PR and main

@bsunderhus bsunderhus merged commit a4410f4 into microsoft:master Oct 4, 2023
@bsunderhus bsunderhus deleted the react-dialog/bugfix/removes-context-hook-from-styles-hook branch October 4, 2023 19:00
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 4, 2023
* master: (35 commits)
  bugfix(react-dialog): removes context hooks invocations from styles hook (microsoft#29396)
  chore(react-dialog): exports DialogSurface context types and hooks (microsoft#29397)
  Minimum height feature for bar chart (microsoft#29359)
  (feat) Breadcrumb - register items via context (microsoft#29393)
  fix: Consider all parents as scroll parents (microsoft#29378)
  Changes to enable charting on fluent docsite (microsoft#29210)
  bugfix: ensure TreeItem emits events properly (microsoft#29390)
  chore(deps): bump get-func-name from 2.0.0 to 2.0.2 (microsoft#29320)
  chore: migrate from getNativeElementProps to getIntrinsicElementProps (microsoft#29387)
  applying package updates
  applying package updates
  fix: Icon in disabled Button shouldn't change color on hover or pressed (microsoft#29342)
  Squish changes for rebase (microsoft#28705)
  fix: GroupedListV2 scrollToIndex now works correctly (microsoft#29332)
  Fix 27482: ProgressBar has an optional state variable  (microsoft#29366)
  remove a11y test ux from theme designer (microsoft#29379)
  feat: preview release (microsoft#29377)
  Fixed divider in the tooltip (microsoft#29357)
  fix (microsoft#29376)
  docs: Initial documentation pass (microsoft#29372)
  ...
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.

3 participants