-
-
Notifications
You must be signed in to change notification settings - Fork 4.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix various accessibility issues #33259
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The left/right padding needs to be adjusted, currently it’s way too little. It should be visually the same as the top/bottom padding. (Also compare to the select dropdown e.g. for language, where the left/right padding is better but actually a bit too much.)
f0ad5d8
to
65acfd5
Compare
I updated the screenshots to include the new paddings |
@CarlSchwan if this is ready for re-review, please re-request review from Jan-C instead of leaving it blocking |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the comment below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now! :)
Probably also best to wait for nextcloud-libraries/nextcloud-vue#2868 to get merged to adapt the style/size as much as possible to the new view component |
@CarlSchwan still ready to merge then ? it's green! |
65acfd5
to
cb64d90
Compare
f456d3b
to
3593cf1
Compare
3593cf1
to
d2d9ddb
Compare
🤔 looks like we just need to adapt the styles in server/apps/settings/src/components/PersonalInfo/shared/AccountPropertySection.vue Lines 192 to 265 in 38d0419
|
d2d9ddb
to
1b4823d
Compare
Merged by @CarlSchwan Shame on him! |
e6612a0
to
8446bb0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🐘 🐘
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 (we got a nice tour of this yesterday)
This comment was marked as resolved.
This comment was marked as resolved.
This fix a regression with the theming api following the change in webpack bundling introduced in 24 Signed-off-by: Carl Schwan <[email protected]>
Input fields require a 44x44 pixels target size, this makes all the input fields and button use that size. Bonus is that now the input fields and buttons now have the same size as the new vue button and this looks less weird than the previous state with controls of different sizes. See https://www.w3.org/WAI/WCAG21/Understanding/target-size.html Signed-off-by: Carl Schwan <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
- Fix user status dialog - Add label where missing - Move emoji picker inside input field (similar to talk) - Fix selecting an emoji - Fix multiselect - Fix button with confirmation action - Fix some other unrelated dark theme issues - Fix select2 focus - Run npm lint:fix Signed-off-by: Carl Schwan <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
Signed-off-by: Carl Schwan <[email protected]>
8446bb0
to
dfd36e8
Compare
dfd36e8
to
f1ce2c2
Compare
Screenshots: