Conversation
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
Signed-off-by: Hamza <hamzamahjoubi221@gmail.com>
|
Copied over design review from the other pr NimishaAwesome! Looks really nice! Only some small changes. Overall:
In the "Delegation" modal:
In the "Add delegates" modal:
Laura
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 :) LauraAdding 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. |
| } | ||
|
|
||
| /** | ||
| * @NoAdminRequired |
There was a problem hiding this comment.
Please use #[NoAdminRequired] attribute
| * @param int $accountId | ||
| * @return JSONResponse |
There was a problem hiding this comment.
| * @param int $accountId | |
| * @return JSONResponse |
Please remove redundant (=> dont provide any additional knoweldge to the type hints) phpdoc
| public function delegate(int $accountId, string $userId): JSONResponse { | ||
|
|
||
| $account = $this->accountService->findById($accountId); | ||
| if ($this->currentUserId === null) { |
| #[TrapError] | ||
| public function delegate(int $accountId, string $userId): JSONResponse { | ||
|
|
||
| $account = $this->accountService->findById($accountId); |
There was a problem hiding this comment.
Would recommend find with try+catch
Mvp for #12403