Vendor review approval modal content edit and close button fix #1206
Conversation
There was a problem hiding this comment.
I deleted this while renaming because it was just a stub without meaningful customization and we aren't using stories as far as I understand.
There was a problem hiding this comment.
That seems fine.
Part of me has felt keeping the storybook setup to be unnecessary. Historically, it hasn't gotten much traffic during development time and that's also reflected currently with only 2 storybook components being defined. Some only being created after the fact, so lacking for prototyping and certainly not much created after component creation so not much done in the way of functionalliy documenting components' usages. I'm all for the concept but not when there isn't any strong support for it -- right now at least.
Another thing is this project's versions are severely outdated and should probably get some updates!
| )} | ||
| {confirmationModal && confirmationModal} |
There was a problem hiding this comment.
This is technically unnecessary but I thought it helped convey the possible of a no render for the modal.
There was a problem hiding this comment.
Yea, thinking you don't have to make conveying this clear but {!!confirmationModal && confirmationModal} could also display that?
stalgiag
changed the title
| title={ | ||
| <div className="review-confirmation-title"> | ||
| <FontAwesomeIcon | ||
| icon={faCheck} |
There was a problem hiding this comment.
Thinking an orange info circle or so could quickly reflect that this is the "non-final" path. I still thought I had incorrectly selected "Approved" instead of "Not Approved" at the end.
Just a thought, what do you think?
There was a problem hiding this comment.
The green check felt odd to me too but I wasn't confident enough about what sentiment we wanted to convey about 'not approved' reviews. Now that I know it feels odd to you as well, I'll swap it out.
There was a problem hiding this comment.
This was challenging. I decided that the exclamation triangle felt most accurate after trying a few others including the info circle. That change is in this commit but I don't feel attached to that decision. That particular "i" just felt off for a modal that didn't have information but instead reflected the outcome of a decision. I felt that the exclamation mark conveyed the need for attention without being too severe since it is orange. Another option is a pause to indicate the process is on hold or waiting further approval.
Here are some screenshots. Do you have a preference?
modal with orange info i in circle
modal with orange triangle exclamation
modal with info i with no circle
There was a problem hiding this comment.
I'm also not strongly tied to either (except the 'i' with no background). Moving forward with the triangle seems fine to me! But thanks so much for the thought you put into that
see #1202
see #1203