Conversation
|
phpcs fails not due to my code changes. Seems to be the same issue in other PRs |
|
@martialblog Please rebase. |
26f476e to
74f9ac2
Compare
|
@lippserd Branch is rebased. |
jrauh01
left a comment
There was a problem hiding this comment.
Thanks for the PR - overall this looks good! The adjustment of the cancel icon is consistent.
Two things to address:
- Role removal button alignment: The confirmation button is currently right-aligned. It should be left-aligned to stay consistent with the other confirmation buttons.
- Dashboard pane removal: The dashlet removal was updated, but the removal of whole dashboard panes wasn't. That'll need to be covered as well.
Did you overlook this one, or are you on it? |
|
Yes, unfortunately the forms are all implemented slightly different. But The |
|
@martialblog I just noticed I overlooked, that the button to remove user groups is right-aligned as well. Your fix won't apply cleanly in this case, and I'm not quite sure if it has to be left-aligned, because of the form description. What do you think about that? |
|
I'm not a certified UX expert so honestly I can't say. We could also probably change the form description to be an HTML element outside the form and align the button. For me personally it's OK as is and since the description relates to the button it probably best to keep them together e.g. for screenreaders. |
|
For me it's fine to keep as is as well. |
- Normalize use of removal icons - Fix tabs in remove pane controller and remove form title - Set Role removal button left alignment
cdc20f5 to
ccedda8
Compare
This PR normalized the use of the phrase "Confirm Removal" in forms that confirm removal.
It also normalizes the use of the "cancel" icon.
refs #5428