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

#8820 render confirm delete button on delete group click #8831

Merged
merged 8 commits into from
Aug 4, 2022

Conversation

sekmiller
Copy link
Contributor

What this PR does / why we need it: Fixes a bug where pressing "enter" on the create group popup deletes the group because it invokes the confirm delete popup continue button

Which issue(s) this PR closes:

Closes #8820 CRITICAL pressing entet in Group Members assignment deletes the Group

Special notes for your reviewer:

Suggestions on how to test this: try adding a group and press enter while the cursor is in the user text box. the group should be created - not deleted

Does this PR introduce a user interface change? If mockups are available, please link/include them here: no

Is there a release notes update needed for this change?:no

Additional documentation:none

@qqmyers
Copy link
Member

qqmyers commented Jul 5, 2022

FWIW: When I first read this issue, I assumed it was similar to #7895 and that a fix like #8500 would be required. Is the current PR still allowing the default button on the page to be clicked and just stopping that from having an effect (versus avoiding the keypress from triggering the action at all)? Or are these not instances of the same base problem?

@sekmiller
Copy link
Contributor Author

In this case according to Philipp, the preferred response to the enter key is to submit and save the group. In that other case since it was the whole dataset page, they wanted enter to be disabled.

@pdurbin pdurbin self-assigned this Jul 28, 2022
Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

I ran the code and it seems to do the job. Crazy that hitting enter was deleting groups!

A couple things I'll note:

  • On develop (4e65f2f), in addition to the "Create Group" dialog I was also able to reproduce the "hit enter to delete" but in the "edit group" dialog. That is, got to "Group Members" and hit enter.
  • Now hitting enter doesn't delete the group. Great! Perhaps it should trigger "Save Changes"? But this can wait for a future PR, I'd say.

@pdurbin pdurbin removed their assignment Jul 28, 2022
@pdurbin
Copy link
Member

pdurbin commented Jul 28, 2022

The Jenkins tests were passing but I went ahead and refreshed from develop.

@coveralls
Copy link

coveralls commented Jul 28, 2022

Coverage Status

Coverage decreased (-0.002%) to 19.725% when pulling d93f771 on 8820-group-deleted-on-enter into 7c1039f on develop.

@kcondon kcondon self-assigned this Aug 3, 2022
@kcondon kcondon merged commit ced92e8 into develop Aug 4, 2022
@kcondon kcondon deleted the 8820-group-deleted-on-enter branch August 4, 2022 15:42
@pdurbin pdurbin added this to the 5.12 milestone Aug 26, 2022
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.

CRITICAL: Pressing Enter in Group Members assignment windows deletes the whole group
5 participants