OH2-443 | Refactor admin forms reset/cancel actions #706
Conversation
There was a problem hiding this comment.
The 'cancel button' could manage at the top level, I mean by the parent component (just like we're doing with the form title). This way, we keep this logic centralize and easy to maintain.
Furthermore, the 'cancel button' actually looks like the button for the 'primary action' of the form, when it's for a secondary or even tertiary action. We could use the secondary color (grey, I think) or a less expressive one.
Finally for the icon, I suggest to use the chevron left or the arrow left
@SilverD3, I agree with the refractoring and the styles. If the cancel/reset operation are OK, then let's move that refecatoring and styles in a separate issue(that will also inclue laboratories one). |
|
It doesn't make much sense that when I click DISCARD, the popup window is called "Cancel" and the OK (primary) button is for proceeding with the discard while the DISCARD (secondary) is to "discard the discard"... let's use "Back to edit" that is more compliant with what the button does:
Moreover, I think the above message should be shown also when the user tries to change page before saving:
On the other hand, the RESET button, shows a popup correctly named "Reset", with a primary OK button for proceeding, and a "Back" (secondary) button for "canceling the resetting", that at least is more related with a "back" operation, but I would call it also here "Back to edit".
So in general in the popup:
For the page itself, I agree with @SilverD3 that the "Discard" should not appear like a primary at the top, but I don't have better suggestions about it at the moment |
Hi @mwithi ! I've just updated labels as suggested ! |
Hi @SteveGT96 can you make also that if I was editing and I try to change page it triggers automatically the discharge event with related popup? |
|
Let me check that ! |
Hi @mwithi ! Could you review it again ? |
I don't see changes yet about it:
|
Yes! Well done! I think we can merge this! One questions: this works only for the admin section, right? what about all other forms in other sections? |
We could handle other parts in separate issues ! |
Sure, what I'm asking is that they have different logic or the same. |
|
I think actually other parts don"t have the same logic like the one in admin section. Should they have the same behaviour/logic ? If yes then we'll hande that in separated issues. |
Still reviewing. There are a lot of changed file |
There was a problem hiding this comment.
I still believe that the "cancel form edit" logic can be centralized and manage at a top level, instead of implementing the same logic in all "form components" of the admin section.
I notice that all pages of the admin section are rendered in the component AdminActivityContent (src\components\activities\adminActivity\AdminActivityContent\AdminActivityContent.tsx). We can centralize the "form cancel" logic in this component, and if not suitable, we can still look for a "higher level" component.
This way, we avoid a lot of code duplication.
| cy.dataCy("close-dialog").click(); | ||
| cy.dataCy("dialog-info").should("not.exist"); | ||
| }); | ||
|
|
||
| it("should cancel the disease creation", () => { | ||
| cy.dataCy("cancel-form").click(); | ||
| cy.dataCy("approve-dialog").click(); | ||
| cy.dataCy("approve-dialog").click().click(); |
There was a problem hiding this comment.
Is this a normal behavior? I mean, clicking twice?
See OH2-443