Conversation
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
3831f1a to
2afc85a
Compare
nimishavijay
left a comment
There was a problem hiding this comment.
Awesome! Looks really nice! Only some small changes. Overall:
- The icon for delegation (across the board) can be supervisor account. The one you've chosen is also very similar but we use that for indicating users and groups
- Instead of "Delegation", I'm wondering if we can use the wording "Delegate account" in the 3 dot menu and the modal title
- Use tertiary buttons for all the "Cancel" buttons and don't use the icon.
In the "Delegation" modal:
- There is some whitespace on top which shouldn't be there (somewhow not there in the "Add delegates" modal)
- We can also center align the "Delegates" heading. Ik in @kra-mo's mockups it was left aligned, but because in the component we always have the heading in the center it looks inconsistent now.
- I see that there's a
h3 { font-weight: bold; }which makes the "Delegates" heading stronger than its parent heading. We could remove that
In the "Add delegates" modal:
- the select component should be full width
I was going off of form group components, not a dialog, in this context, I think the two different headings don't make sense either, we could just have one "Delegates" title :) |
There was a problem hiding this comment.
Adding to what @nimishavijay said, the "Revoke" button should also probably not have an icon, a checkmark with danger styling is always confusing :)
Additionally, the user selection dropdown should be full-width and there should be a bit more padding between the explanation text beneath and the buttons, also in the revoke access dialog.
STATUS: I'll wait for the backend to get merged before continuing with this
requires #12605
ref #12403