Skip to content

divided report dialog and report icon#1178

Open
AnnieeBennie wants to merge 6 commits into
masterfrom
ReportArticleDivision
Open

divided report dialog and report icon#1178
AnnieeBennie wants to merge 6 commits into
masterfrom
ReportArticleDivision

Conversation

@AnnieeBennie

Copy link
Copy Markdown
Collaborator

+divided report dialog and report icon
+renamed ReportBroken => ReportBrokenArticle
+created a separate file for report dialog styles

@netlify

netlify Bot commented Jun 23, 2026

Copy link
Copy Markdown

Deploy Preview for voluble-nougat-015dd1 ready!

Name Link
🔨 Latest commit 2b10c55
🔍 Latest deploy log https://app.netlify.com/projects/voluble-nougat-015dd1/deploys/6a44f565632ba00008ea2927
😎 Deploy Preview https://deploy-preview-1178--voluble-nougat-015dd1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@mircealungu mircealungu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Best-decomposed of the set — clean container/presentational split, and I confirmed the prop wiring keeps behavior identical. ReportDialog's defaults also make it reusable for a future "report video", nice. Three small things:

  1. .sc.js naming — note these are MUI sx objects (one's a (theme) => callback), so they can't be styled-components like the rest of the repo's .sc.js. Simplest fix is to rename the file to ReportDialog.styles.js so it's not mislabeled.
  2. ReportIcon duplicates SettingsIconButton (from #1180) — same padding/cursor/flex wrapper + #999 icon, only the glyph differs. Worth collapsing the two into one shared IconButton that takes an icon + ariaLabel instead of two near-identical files.
  3. Tiny pre-existing bug you're inheriting: flexDisplay: "row" in reportDialogContentStyles isn't a real property (no-op) — drop it, or make it flexDirection if a column was actually intended.

Otherwise 👍

@AnnieeBennie

Copy link
Copy Markdown
Collaborator Author

@mircealungu done!

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.

2 participants