-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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
fix: clear 'delete' confirmation #17345
Conversation
Codecov Report
@@ Coverage Diff @@
## master #17345 +/- ##
=======================================
Coverage 77.13% 77.14%
=======================================
Files 1036 1036
Lines 55729 55740 +11
Branches 7627 7628 +1
=======================================
+ Hits 42989 42998 +9
- Misses 12484 12486 +2
Partials 256 256
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
||
const onChange = (event: React.ChangeEvent<HTMLInputElement>) => { | ||
const targetValue = event.target.value ?? ''; | ||
setDisableChange(targetValue.toUpperCase() !== t('DELETE')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is a tricky thing.. so we would need to have a translation for the capitalized text and the lowercase?
I wonder if t('delete').toUpperCase()
would work. (I see that this isn't new code, so it's prob fine to leave as is. It just got me thinking.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eschutho this used to be much worse... before it didn't have the t()
call, so we'd ask people in a different language, Por favor digite "apagar" para confirmar, but expected "delete" to unblock the dialog...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ha, yeah I remember when you noticed that. :)
* fix: clear 'delete' confirmation * Add tests (cherry picked from commit 43f4ab8)
* fix: clear 'delete' confirmation * Add tests
SUMMARY
Currently
<DeleteModal/>
does not clear the "delete" confirmation after a deletion. This means that:I changed the component so that the text in the
<Input/>
component is managed, clearing it when the modal is hidden or after a delete.I also changed it so that users can press "return" to delete, once they've confirmed, instead of forcing users to click the button.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
bulk_delete_before.mov
After:
bulk_delete_after.mov
TESTING INSTRUCTIONS
Adding tests.
ADDITIONAL INFORMATION