Skip to content

chore(#588): extract shared dialog styles#704

Open
latin-panda wants to merge 2 commits intomainfrom
refactor-dialog-styles
Open

chore(#588): extract shared dialog styles#704
latin-panda wants to merge 2 commits intomainfrom
refactor-dialog-styles

Conversation

@latin-panda
Copy link
Collaborator

@latin-panda latin-panda commented Mar 3, 2026

Closes #588

I have verified this PR works in these browsers (latest versions):

  • Chrome
  • Firefox
  • Safari (macOS)
  • Safari (iOS)
  • Chrome for Android
  • Not applicable

What else has been done to verify that this works as intended?

Expand to see before (right) and after (left) the change.

Why is this the best possible solution? Were any other approaches considered?

  • Creating a custom OdkDialog wrapper adds unnecessary boilerplate (manually passing slots, props, and emitted events) with minimal structural benefit, as each dialog still requires unique internal layouts for the header, body, and footer.

  • We leveraged odkThemePreset to globally apply the dialog "skin".

  • For those other common styles that cannot be defined in the preset, they are extracted into the global primevue.scss.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Nothing

Do we need any specific form for testing your changes? If so, please attach one.

No

What's changed

  • Updates the dialog's radius and background in the preset.
  • Moves common styles to primevue.scss

@changeset-bot
Copy link

changeset-bot bot commented Mar 3, 2026

⚠️ No Changeset found

Latest commit: 85288d6

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@latin-panda latin-panda requested a review from garethbowen March 3, 2026 09:10
@latin-panda
Copy link
Collaborator Author

@garethbowen Whenever you get a moment, could you please review this? Thank you so much!

Copy link
Collaborator

@garethbowen garethbowen left a comment

Choose a reason for hiding this comment

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

So nice to clean this up, thanks!

One suggestion about the right cursor for disabled elements, but approved to unblock.

.p-datepicker-input:disabled + .p-datepicker-input-icon-container,
.p-inputtext:disabled,
.p-inputtext:read-only,
.p-button:disabled {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is cursor: not-allowed the right style here? I think that it's more for things where you don't have permission, and we should use cursor: default instead. What do you think?

}

.p-button:disabled {
cursor: not-allowed;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ditto.

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.

Reuse dialog styles

2 participants