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

Add confirmation modal when removing hero with saved builds #209

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tyopoyt
Copy link
Contributor

@tyopoyt tyopoyt commented Sep 26, 2023

No description provided.

@tyopoyt tyopoyt changed the title Remove hero confirmation Add confirmation modal when removing hero with saved builds Sep 26, 2023
@jmferreirab
Copy link
Contributor

jmferreirab commented Sep 27, 2023

Thank you for the pr @tyopoyt. Please try to add a description and/or visuals of the change to make the PR easier to review.

@fribbels I think this is a nice QoL. Useful when someone manages to accidentally hit remove hero on a hero they have a lot of builds. Likewise, it is not intrusive enough to be annoying since the extra confirmation is only being added to a rarely used button, similar to Github’s delete repository button.

Tested it myself and it's working fine.

image

@tyopoyt
Copy link
Contributor Author

tyopoyt commented Sep 27, 2023

jmferreirab Ah yeah, I'll include more details in the pr body next time. I may have been somebody who managed to hit delete on a unit with builds I was working on 😅 Was trying to remove a build that I had ruled out

Houdeeny added a commit to Houdeeny/Fribbels-Epic-7-Optimizer that referenced this pull request Jun 14, 2024
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.

2 participants