Skip to content

chore(web-components): refactor fluent-dialog and add new fluent-dialog-body component#31512

Merged
chrisdholt merged 20 commits into
microsoft:masterfrom
davatron5000:davatron5000/refactor-fluent-dialog
Jun 4, 2024
Merged

chore(web-components): refactor fluent-dialog and add new fluent-dialog-body component#31512
chrisdholt merged 20 commits into
microsoft:masterfrom
davatron5000:davatron5000/refactor-fluent-dialog

Conversation

@davatron5000
Copy link
Copy Markdown
Contributor

@davatron5000 davatron5000 commented May 29, 2024

Previous Behavior

  • The initial implementation of <fluent-dialog> had a lot of duplicate functionality with native HTMLDialogElement
  • Didn't support content scenarios like a full-width image in the Dialog Body
  • No tests for dialog

New Behavior

  • Cleanup of Dialog component and internal markup
  • Remove unnecessary focus trap behavior
  • Remove open attribute functionality
  • Beef up stories and supported scenarios
  • Rename modalType attribute to type
  • 🆕 Allow for non-padded content like a fulll width image inside dialog
  • 🆕 Add CSS transitions to dialog open/close
  • 🆕 Add <fluent-dialog-body> element to handle default content behavior
  • 🆕 Begin to add tests for <fluent-dialog>

Related Issue(s)

  • Fixes #

@fabricteam
Copy link
Copy Markdown
Collaborator

fabricteam commented May 29, 2024

📊 Bundle size report

✅ No changes found

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented May 29, 2024

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.

@davatron5000 davatron5000 marked this pull request as ready for review May 29, 2024 20:23
@davatron5000 davatron5000 requested a review from a team as a code owner May 29, 2024 20:23
Comment thread packages/web-components/src/dialog/dialog.styles.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.template.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts Outdated
Copy link
Copy Markdown
Member

@chrisdholt chrisdholt left a comment

Choose a reason for hiding this comment

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

This looks like a great improvement - a couple questions and comments to see if we can trim/simplify/align in a few places

Comment thread packages/web-components/src/dialog-body/dialog-body.template.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.template.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.template.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.styles.ts
Comment thread packages/web-components/src/dialog/dialog.styles.ts
Comment thread packages/web-components/src/dialog/dialog.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.template.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.ts Outdated
Comment thread packages/web-components/src/dialog-body/define.ts
Comment thread packages/web-components/src/dialog-body/dialog-body.definition.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts
Comment thread packages/web-components/src/dialog-body/dialog-body.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.ts Outdated
@davatron5000 davatron5000 force-pushed the davatron5000/refactor-fluent-dialog branch from 2d11400 to f20fe20 Compare May 31, 2024 20:35
Copy link
Copy Markdown
Contributor

@radium-v radium-v left a comment

Choose a reason for hiding this comment

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

The reduction in class methods is incredible to see here, great work!

I'd love to see some more Playwright tests, as well as some duplication cleanup in the story files if that's possible.

Comment thread packages/web-components/src/dialog-body/dialog-body.stories.ts Outdated
Comment thread packages/web-components/src/dialog-body/dialog-body.styles.ts
Comment thread packages/web-components/src/dialog-body/dialog-body.template.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.spec.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.spec.ts Outdated
Comment thread packages/web-components/src/dialog/dialog.ts Outdated
@davatron5000 davatron5000 force-pushed the davatron5000/refactor-fluent-dialog branch from c32edae to d08c0b1 Compare June 3, 2024 22:01
@chrisdholt chrisdholt merged commit 1084cb6 into microsoft:master Jun 4, 2024
miroslavstastny pushed a commit to miroslavstastny/fluentui that referenced this pull request Jun 14, 2024
…og-body component (microsoft#31512)

Co-authored-by: Jane Chu <7559015+janechu@users.noreply.github.com>
Co-authored-by: John Kreitlow <863023+radium-v@users.noreply.github.com>
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.

6 participants