Skip to content

Delete account: design improvements#2682

Open
nqst wants to merge 16 commits intobasecamp:mainfrom
nqst:delete-account-improvements
Open

Delete account: design improvements#2682
nqst wants to merge 16 commits intobasecamp:mainfrom
nqst:delete-account-improvements

Conversation

@nqst
Copy link
Contributor

@nqst nqst commented Mar 10, 2026

Changes

Account Settings page

  • Updated the copy in the Cancel account section, making it more informative.
  • Added ... to the button label to indicate that confirmation is required and to keep it consistent with the Begin export... button.
  • Changed the heading horizontal line color to light red.
  • Previously, both Cancel and Delete terms were used. I wanted to stick with one and chose Cancel. But Delete looked good to me as well.

Cancel Account dialog

  • Added a confirmation checkbox as an extra safety measure.
  • Added a suggestion to export the data first (took the idea from Basecamp).
  • Specified the name of the account that is going to be canceled.

Preview

Before After
settings-before@2x settings-after@2x
dialog-before@2x dialog-after@2x

Notes

CSS

  • I extracted the .checkbox styles from .step__checkbox (moved from steps.css to inputs.css). There are no visual changes.
  • --hover-color: var(--card-color) was unused so I removed it.
  • .list-unindented aligns bullets with the regular text.

JS

  • The dispatch_event controller is used to open the Export dialog from within the Cancellation dialog.
  • The new reset() method in toggle_enable controller is used to reset the Cancellation dialog state after closing.

Copilot AI review requested due to automatic review settings March 10, 2026 09:37
Copy link
Contributor

Copilot AI left a comment

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 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__checkbox CSS into a shared .checkbox class in inputs.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.js for dispatching named DOM events from document, and extended toggle_enable_controller.js with a reset() 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
@flavorjones flavorjones requested a review from andyra March 18, 2026 16:10
Copy link
Contributor

@andyra andyra left a comment

Choose a reason for hiding this comment

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

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 delete Delete your account and everything in it…
  • Button: Cancel Delete account…
  • Modal header: Cancel Delete account
  • Modal button: Cancel Delete my account

Copilot AI review requested due to automatic review settings March 19, 2026 16:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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? } %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

@nqst
Copy link
Contributor Author

nqst commented Mar 19, 2026

@andyra thanks!

I've just changed "Cancel" to "Delete" in 754e2e3 👌

the account data isn't hanging around somewhere else after you cancel

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):

Screenshot 2026-03-19 at 15 28 08@2x

That said, I think "Delete" works too! But if you change your mind, we can revert.

@nqst nqst requested a review from andyra March 19, 2026 16:47
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.

3 participants