Skip to content

[73372] Wrong icon used when changing non working days - #23292

Merged
bsatarnejad merged 7 commits into
devfrom
73372-wrong-icon-used-when-changing-non-working-days
May 29, 2026
Merged

[73372] Wrong icon used when changing non working days#23292
bsatarnejad merged 7 commits into
devfrom
73372-wrong-icon-used-when-changing-non-working-days

Conversation

@bsatarnejad

Copy link
Copy Markdown
Contributor

Ticket

https://community.openproject.org/wp/73372

What are you trying to accomplish?

Use a Primer DangerDialog for the working-days confirmation modal.

@bsatarnejad bsatarnejad self-assigned this May 20, 2026
@bsatarnejad
bsatarnejad marked this pull request as ready for review May 20, 2026 17:57

@HDinger HDinger left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had some troubles with my database which I think are not related to your changes but that prevented my from testing whether the update actually worked..

Some other things: there seems to be some space missing between the list of deleted dates and the upcoming text:

Image

Comment thread app/components/admin/settings/working_days/confirm_dialog_component.rb Outdated
Comment thread app/components/admin/settings/working_days/confirm_dialog_component.html.erb Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the admin “Working days & hours” confirmation flow to use a server-rendered Primer DangerDialog (via Turbo Streams) instead of the legacy Angular confirm dialog, addressing the wrong-icon issue when changing non-working days.

Changes:

  • Replaces the Angular ConfirmDialogService flow with a Turbo Stream request that renders Admin::Settings::WorkingDays::ConfirmDialogComponent.
  • Adds an Admin::Settings::WorkingDaysAndHoursSettingsController#confirm_changes endpoint + route to serve the dialog content.
  • Updates feature specs to interact with the new dialog and adds component specs for the new dialog component.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
spec/features/admin/working_days_spec.rb Updates feature spec interactions to click Primer dialog buttons instead of the legacy confirmation dialog helper.
spec/components/admin/settings/working_days/confirm_dialog_component_spec.rb Adds component specs for the new DangerDialog, including hidden-field submission behavior.
frontend/src/app/shared/components/op-non-working-days-list/op-non-working-days-list.component.ts Replaces the legacy modal confirm flow with a Turbo Stream request that opens the server-rendered dialog.
config/routes.rb Adds a POST route for working_days_and_hours/confirm_changes.
config/locales/js-en.yml Removes now-unused JS-only confirmation dialog strings.
config/locales/en.yml Adds server-side i18n keys for the new working-days confirmation dialog.
app/controllers/admin/settings/working_days_and_hours_settings_controller.rb Adds confirm_changes action and refactors param access to support the dialog rendering.
app/components/admin/settings/working_days/confirm_dialog_component.rb Introduces a new component to render the confirmation dialog and re-post captured settings via hidden fields.
app/components/admin/settings/working_days/confirm_dialog_component.html.erb Implements the Primer DangerDialog markup/content (no icon) and includes hidden settings fields for submission.

@bsatarnejad
bsatarnejad requested a review from HDinger May 22, 2026 07:33
Comment thread config/locales/en.yml Outdated
@bsatarnejad
bsatarnejad requested a review from HDinger May 26, 2026 10:15
@HDinger HDinger added this to the 17.6.x milestone May 27, 2026
@bsatarnejad
bsatarnejad merged commit 031c3ce into dev May 29, 2026
21 checks passed
@bsatarnejad
bsatarnejad deleted the 73372-wrong-icon-used-when-changing-non-working-days branch May 29, 2026 07:08
@github-actions github-actions Bot locked and limited conversation to collaborators May 29, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Development

Successfully merging this pull request may close these issues.

3 participants