#766 Show prompt only if form value changed #1086
Conversation
When leaving the howto edit page a confirmation dialog is shown to warn the user that he/she might loose the changes. This commit changes this logic so that the dialog is only shown when there are actual changes that would be lost. The dirty state of the form is used for this. A big part of the added logic is for correctly marking a field (and thus the form) as dirty.
Add the display of a confirm dialog on a complete page refresh/reload not only on a React Router change.
|
Test summary
Run details
View run in Cypress Dashboard ➡️ This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard |
|
|
||
| // Functions used to give as callback to the isEqual prop of a form fields. | ||
| // The isEqual callback is used to determine if a field is dirty. | ||
| export const COMPARISONS = { |
There was a problem hiding this comment.
Really nice to have these defined in a reusable way, will come in handy elsewhere for sure
| it('[Warning on leaving page]', () => { | ||
| const stub = cy.stub() | ||
| stub.returns(false); | ||
| cy.on('window:confirm', stub) |
There was a problem hiding this comment.
nice idea to stub this, wouldn't have thought of that!
| @@ -228,6 +251,7 @@ export class HowtoForm extends React.PureComponent<IProps, IState> { | |||
| ? this.validateTitle(value) | |||
| : false | |||
| } | |||
| isEqual={COMPARISONS.textInput} | |||
There was a problem hiding this comment.
I feel like adding comparisons to every field might be a bit time-intensive to manage if this form were to change or if we want to apply the same to other forms (e.g. creating events). Although I'm guessing processing at the form level is then computationally quite a bit more intensive (checking every field any time anything changes).... So I think probably the best solution is using a better form library that has more native support for this, and for now we have the option to apply at the form level if we want.
There was a problem hiding this comment.
I added a few inline comments and a bit more context to the discussion on #766 , but overall I think the additions are great and my own testing seemed to work well. Merging.
1086-Testing-1.mp4
PR Type
PR Checklist
masterbranch mergedDescription
Only show the confirm dialog (prompt) when a value is changed inside the howto edit/create page
Git Issues
#766
Screenshots/Videos