matrix-react-sdk
matrix-react-sdk copied to clipboard
Add padding for close button in settings dialog
Signed-off-by: gefgu [email protected]
Checklist
- [ ] Tests written for new code (and old code if feasible)
- [x] Linter and other CI checks pass
- [X] Sign-off given on the changes (see CONTRIBUTING.md)
Notes: Add padding for close button in settings dialog. It maintains the alignment with the heading, maintains the original size of the "X" mark and follows Apple's human interface guidelines of 44x44 points for touch targets. It fixes https://github.com/vector-im/element-web/issues/22915
Type: [enhancement/defect/task]
Before:
After:
Here's what your changelog entry will look like:
🐛 Bug Fixes
- Add padding for close button in settings dialog. It maintains the alignment with the heading, maintains the original size of the "X" mark and follows Apple's human interface guidelines of 44x44 points for touch targets. It fixes https (#9202). Contributed by @gefgu.
I didn't let the button in the center of the clickable area because it would lose the alignment with the heading, but if you prefer it to the center, I will work on it.
I've requested a review from the design team to figure out how this should actually look/behave
Thanks for the quick feedback! When they figure out, I'm happy to help.
I'd suggest using a target area of 32x32px for the desktop context.
- It's a nice trade off between not taking way too much space and complying with upcoming WCAG 2.2 recommendation for AA level (24x24px).
- I'd only recommend using 44x44px when in a touch context (mobile viewport, webview, touch input...), or trying to comply with WCAG 2.2 AAA.
As for the icon alignment and button placement I'd suggest, for now:
- Keep the close icon centered within the 32x32px target area.
- Place the 32x32px target area at 24px from the the modal window edges.
I've updated the code following @janogarcia suggestion.
The close icon is centered within the 32x32 target area:
And the target area is at 24px from the the modal window edges:
I hope this solve the issue. What do you think @SimonBrandner?
The playwright tests are failing due to a difference in the size of the close icons - https://e2e-9202--matrix-react-sdk.netlify.app/#?testId=e020cd10d103304ed79f-bd3d48af8331ff4cee52038c2392796d4a22. I don't think this PR intended to change the size of the close button in the first place.
Closing this for now, feel free to reopen if you want to make the changes.