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

Script Edit Page: Add confirmation dialog when deleting lesson #37649

Merged
merged 5 commits into from Nov 5, 2020

Conversation

dmcavoy
Copy link
Contributor

@dmcavoy dmcavoy commented Nov 3, 2020

Bethany pointed out during the bug bash that we did not have a confirmation dialog on the delete of a lesson on the script edit page. This adds a dialog to confirm you want to delete the lesson.

remove-lesson

Links

Testing story

Added a test for the new dialog.

Reviewer Checklist:

  • Tests provide adequate coverage
  • Privacy and Security impacts have been assessed
  • Code is well-commented
  • New features are translatable or updates will not break translations
  • Relevant documentation has been added or updated
  • User impact is well-understood and desirable
  • Pull Request is labeled appropriately
  • Follow-up work items (including potential tech debt) are tracked and linked

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

Good catch!


render() {
const {lessonName, handleClose, lessonPosToRemove} = this.props;
let bodyText = `Are you sure you want to remove the level named "${lessonName}" from the script?`;
Copy link
Member

Choose a reason for hiding this comment

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

Sorry to nit, but there is a lot at stake if the user sees this warning, soI think this could benefit from a stronger warning message, like:

big warning: "Are you sure you want to permanently delete the lesson named X and all its contents?"

optional smaller text: "This will delete any details within the lesson, such as level progressions and lesson plan markdown. Individual levels will not be deleted."

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2020-11-03 at 4 02 38 PM

@davidsbailey how is this?

Copy link
Member

@davidsbailey davidsbailey left a comment

Choose a reason for hiding this comment

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

👍 looks great!

Copy link
Contributor

@bethanyaconnor bethanyaconnor left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this!

@dmcavoy dmcavoy merged commit 70a5913 into staging Nov 5, 2020
@dmcavoy dmcavoy deleted the remove-lesson-dialog branch November 5, 2020 22:14
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.

None yet

3 participants