Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#60359] Harmonise FeedbackDialog slot naming #219

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

myabc
Copy link

@myabc myabc commented Jan 10, 2025

What are you trying to accomplish?

Renames the additional_content slot to additional_details to make Feedback Dialog consistent with the newly introduced Danger Confirmation Dialog.

ViewComponent defines a with_SLOT_NAME_content method for given slots. Using additional_content as a slot name results in a with_additonal_content_content method being generated - this makes for a poor API for component users.

Screenshots

End users should not see any visual changes.

Integration

This will require update of opf/openproject See PR: opf/openproject#17568

List the issues that this change affects.

Closes https://community.openproject.org/work_packages/60359

Risk Assessment

  • Low risk the change is small, highly observable, and easily rolled back.
  • Medium risk changes that are isolated, reduced in scope or could impact few users. The change will not impact library availability.
  • High risk changes are those that could impact customers and SLOs, low or no test coverage, low observability, or slow to rollback.

What approach did you choose and why?

Anything you want to highlight for special attention from reviewers?

Accessibility

  • Fixes axe scan violation - This change fixes an existing axe scan violation.
  • No new axe scan violation - This change does not introduce any new axe scan violations.
  • New axe violation - This change introduces a new axe scan violation. Please describe why the violation cannot be resolved below.

Merge checklist

  • Added/updated tests
  • Added/updated documentation
  • Added/updated previews (Lookbook)
  • Tested in Chrome
  • Tested in Firefox
  • Tested in Safari
  • Tested in Edge

Copy link

changeset-bot bot commented Jan 10, 2025

🦋 Changeset detected

Latest commit: dae6eae

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@openproject/primer-view-components Minor

Not sure what this means? Click here to learn what changesets are.

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

Renames the `additional_content` slot to `additional_details` to make
Feedback Dialog consistent with the newly introduced Danger Confirmation
Dialog.

ViewComponent defines a `with_SLOT_NAME_content` method for given slots.
Using `additional_content` as a slot name results in a
`with_additonal_content_content` method being generated - this makes for
a poor API for component users.
Copy link
Collaborator

@HDinger HDinger left a comment

Choose a reason for hiding this comment

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

Please add a changeset and fix the CI. Otherwise good 👍

@myabc myabc force-pushed the feature/60359-update-feedback_dialog-component-slots branch from 3e38519 to a1b641c Compare January 10, 2025 13:06
@HDinger HDinger force-pushed the feature/60359-update-feedback_dialog-component-slots branch from ef42f27 to d11d468 Compare January 10, 2025 15:39
myabc and others added 3 commits January 10, 2025 12:52
Makes naming of Feedback Dialog preview templates consistent with those
used by newly introduced Danger Confirmation Dialog.
@myabc myabc force-pushed the feature/60359-update-feedback_dialog-component-slots branch from d11d468 to dae6eae Compare January 10, 2025 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants