Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the design of the account deletion/cancellation flow in account settings. It consolidates the custom checkbox styling into a reusable .checkbox CSS class (previously .step__checkbox), adds a confirmation checkbox and better-worded copy to the cancellation dialog, introduces a new dispatch_event Stimulus controller for cross-dialog communication, and adds a reset() method to the existing toggle_enable controller.
Changes:
- Extracted
.step__checkboxCSS into a shared.checkboxclass ininputs.css, replacing all usages across step-related views. - Redesigned the cancellation dialog with a confirmation checkbox, improved copy, and a link to trigger the export dialog from within the cancellation dialog.
- Added a new
dispatch_event_controller.jsfor dispatching named DOM events fromdocument, and extendedtoggle_enable_controller.jswith areset()method to restore dialog state on close.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
app/assets/stylesheets/inputs.css |
Adds the extracted .checkbox component styles |
app/assets/stylesheets/steps.css |
Removes now-extracted .step__checkbox styles |
app/assets/stylesheets/settings.css |
Adds .settings__section--negative modifier for danger zone styling |
app/assets/stylesheets/utilities.css |
Adds .list-unindented utility class |
app/views/cards/steps/_step.html.erb |
Updates checkbox class reference from .step__checkbox to .checkbox |
app/views/cards/steps/edit.html.erb |
Updates checkbox class reference |
app/views/cards/display/perma/_steps.html.erb |
Updates checkbox class reference |
app/views/public/cards/show/_steps.html.erb |
Updates checkbox class reference |
app/views/account/settings/_cancellation.html.erb |
Redesigns cancellation dialog with confirmation checkbox, improved copy, and export link |
app/views/account/settings/_export.html.erb |
Adds open-export@document event listener to enable opening from cancellation dialog |
app/javascript/controllers/dispatch_event_controller.js |
New controller for dispatching named events from document |
app/javascript/controllers/toggle_enable_controller.js |
Adds reset() method and checkbox target support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Addresses Copilot's feedback
andyra
left a comment
There was a problem hiding this comment.
Really nice work, @nqst. 👏 It was smart to abstract the step__checkbox class as well.
The one thing I'd suggesting changing is "Cancel account" language. I'd stick with "Delete", as that more strongly suggests that the account data isn't hanging around somewhere else after you cancel it. You do a good job of explaining it in the modal, but we may as well be direct about it in the header/button text as well.
- Section description:
Cancel your account and deleteDelete your account and everything in it… - Button:
CancelDelete account… - Modal header:
CancelDelete account - Modal button:
CancelDelete my account
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| <% card.steps.each do |step| %> | ||
| <li class="step"> | ||
| <%= check_box_tag :completed, { class: "step__checkbox", disabled: true, checked: step.completed? } %> | ||
| <%= check_box_tag :completed, { class: "checkbox", disabled: true, checked: step.completed? } %> |
There was a problem hiding this comment.
This is unrelated to the changes in this PR (I just renamed the class here), but it seems to be a valid bug. I can address it separately if you’d like, or feel free to accept the suggestion if it’s looking good to you.
|
@andyra thanks! I've just changed "Cancel" to "Delete" in 754e2e3 👌
If I understood correctly, the account data is kept for 30 days after the cancellation. Because of that, I was a bit torn between "Delete" and "Cancel", and chose the latter for this PR. It also matches the wording in the cancellation emails (cancellation.html.erb): That said, I think "Delete" works too! But if you change your mind, we can revert. |

Changes
Account Settings page
...to the button label to indicate that confirmation is required and to keep it consistent with the Begin export... button.Cancel Account dialog
Preview
Notes
CSS
.checkboxstyles from.step__checkbox(moved fromsteps.csstoinputs.css). There are no visual changes.--hover-color: var(--card-color)was unused so I removed it..list-unindentedaligns bullets with the regular text.JS
dispatch_eventcontroller is used to open the Export dialog from within the Cancellation dialog.reset()method intoggle_enablecontroller is used to reset the Cancellation dialog state after closing.