Skip to content

Conversation

@bsunderhus
Copy link
Contributor

@bsunderhus bsunderhus commented May 15, 2023

⚠️ This PR will introduce Visual Regressions! as it'll modify the spacing visually available inside a Dialog, but technically this visual regression is a bugfix! As that space should never be there in the first place!

New Behavior

  1. Removes grid-template-* and grid-area css styles since they generate a pre-defined "space" on the grid (meaning that this pre-defined space will introduce gaps around of it)
  2. Uses grid-column-* and grid-row-* css styles to properly position every single area of the grid without generating pre-defined spaces

Related Issue(s)

@size-auditor
Copy link

size-auditor bot commented May 15, 2023

Asset size changes

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

Baseline commit: 30ff8c1f8364df94fc10c4f26c88e495cb44dd2e (build)

@fabricteam
Copy link
Collaborator

📊 Bundle size report

Package & Exports Baseline (minified/GZIP) PR Change
react-dialog
Dialog (including children components)
90.972 kB
27.056 kB
90.204 kB
26.857 kB
-768 B
-199 B
Unchanged fixtures
Package & Exports Size (minified/GZIP)
react-components
react-components: Button, FluentProvider & webLightTheme
64.855 kB
17.852 kB
react-components
react-components: Accordion, Button, FluentProvider, Image, Menu, Popover
203.937 kB
57.086 kB
react-components
react-components: FluentProvider & webLightTheme
36.086 kB
11.9 kB
react-portal-compat
PortalCompatProvider
6.446 kB
2.186 kB
🤖 This report was generated against 30ff8c1f8364df94fc10c4f26c88e495cb44dd2e

@codesandbox-ci
Copy link

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

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration
priceless-hooks-2z3iln Issue #27681

@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 718 682 5000
Button mount 389 376 5000
Field mount 1254 1320 5000
FluentProvider mount 889 904 5000
FluentProviderWithTheme mount 118 118 10
FluentProviderWithTheme virtual-rerender 104 94 10
FluentProviderWithTheme virtual-rerender-with-unmount 107 110 10
InfoButton mount 25 18 5000
MakeStyles mount 1116 1130 50000
Persona mount 2067 1992 5000
SpinButton mount 1563 1578 5000

@bsunderhus bsunderhus marked this pull request as ready for review May 15, 2023 16:59
@bsunderhus bsunderhus requested a review from a team as a code owner May 15, 2023 16:59
@bsunderhus bsunderhus merged commit 37159cd into microsoft:master May 16, 2023
@bsunderhus bsunderhus deleted the react-dialog/bugfix--removes-unnecessary-grid-gaps branch May 16, 2023 08:45
@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

@msft-fluent-ui-bot
Copy link
Collaborator

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request May 18, 2023
* master: (24 commits)
  chore(react-tabster): upgrade tabster to v4.4.2 (microsoft#27540)
  feat(react-tags): Add TagGroup with context (microsoft#27886)
  applying package updates
  fix(react-infobutton): Add aria-owns to InfoLabel (microsoft#27834)
  fix(recipes-react-components): Add a FluentProvider to the local storybook (microsoft#27746)
  chore: update RFC template (microsoft#27880)
  applying package updates
  feat: implement Toaster offset (microsoft#27854)
  feat(react-drawer): create DrawerFooter component (microsoft#27583)
  Make getKey and selection props mutually exclusive (microsoft#24048)
  Added MIGRATION.md to the Breadcrumb (microsoft#27846)
  update Github CODEOWNERS file (microsoft#27849)
  feat(react-tags): make basic Tag a button instead of div (microsoft#27858)
  chore: add test-ssr script to v9 packages (microsoft#27690)
  chore(react-tree): exports TreeItemAside unstable (microsoft#27856)
  bugfix(react-dialog): removes unnecessary grid gaps (microsoft#27845)
  applying package updates
  fix(react-textarea): Don't remove outline when filled and disabled and apply correct disabled color to text (microsoft#27837)
  feat: Implement limit for toast stacking (microsoft#27848)
  Update README.md for fluent 2 theme to include import instructions (microsoft#27847)
  ...
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request May 18, 2023
* feat/drawer-header: (24 commits)
  chore(react-tabster): upgrade tabster to v4.4.2 (microsoft#27540)
  feat(react-tags): Add TagGroup with context (microsoft#27886)
  applying package updates
  fix(react-infobutton): Add aria-owns to InfoLabel (microsoft#27834)
  fix(recipes-react-components): Add a FluentProvider to the local storybook (microsoft#27746)
  chore: update RFC template (microsoft#27880)
  applying package updates
  feat: implement Toaster offset (microsoft#27854)
  feat(react-drawer): create DrawerFooter component (microsoft#27583)
  Make getKey and selection props mutually exclusive (microsoft#24048)
  fix: move style override to outside the component
  Added MIGRATION.md to the Breadcrumb (microsoft#27846)
  update Github CODEOWNERS file (microsoft#27849)
  feat(react-tags): make basic Tag a button instead of div (microsoft#27858)
  chore: add test-ssr script to v9 packages (microsoft#27690)
  chore(react-tree): exports TreeItemAside unstable (microsoft#27856)
  bugfix(react-dialog): removes unnecessary grid gaps (microsoft#27845)
  applying package updates
  fix(react-textarea): Don't remove outline when filled and disabled and apply correct disabled color to text (microsoft#27837)
  feat: Implement limit for toast stacking (microsoft#27848)
  ...
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.

[Bug]: Dialog applies grid gaps for elements which aren't rendered

4 participants