Skip to content
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

Improve multiselect #3100

Merged
merged 2 commits into from
Aug 30, 2022
Merged

Improve multiselect #3100

merged 2 commits into from
Aug 30, 2022

Conversation

CarlSchwan
Copy link
Contributor

@CarlSchwan CarlSchwan commented Aug 25, 2022

  • Add focus effect for better accessibility
  • Increase font size to be 15px like the other component
  • Update style to be more like the NcInputField

There are still some remaining issues that are not fixed by this PR,
notably the width of the multiselect component change when clicking
on it. This is a preexisting issue :(

image

- Add focus effect for better accessibility
- Increase font size to be 15px like the other component
- Update style to be more like the NcInputField

There is still some remaining issues that are not fixed by this PR,
notably the width of the multiselect component change when clicking
on it. This is a preexisting issue

Signed-off-by: Carl Schwan <[email protected]>
@CarlSchwan CarlSchwan added enhancement New feature or request 3. to review Waiting for reviews accessibility Making sure we design for the widest range of people possible, including those who have disabilities labels Aug 25, 2022
@CarlSchwan CarlSchwan added this to the 6.0.0 milestone Aug 25, 2022
@CarlSchwan CarlSchwan self-assigned this Aug 25, 2022
Copy link
Contributor

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
It's kinda strange that the field itself is double height when open, but guess also preexisting issue.

@CarlSchwan
Copy link
Contributor Author

Looks good! It's kinda strange that the field itself is double height when open, but guess also preexisting issue.

Yeah here is a before screenshot

image

src/components/NcMultiselect/index.scss Outdated Show resolved Hide resolved
src/components/NcMultiselect/index.scss Outdated Show resolved Hide resolved
Signed-off-by: Carl Schwan <[email protected]>
Copy link
Contributor

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a quick pass over the code. Looks legit.

@ChristophWurst ChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 30, 2022
@CarlSchwan CarlSchwan merged commit 4c2ecc5 into master Aug 30, 2022
@CarlSchwan CarlSchwan deleted the fix/ncmultiselect-accessibility branch August 30, 2022 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish accessibility Making sure we design for the widest range of people possible, including those who have disabilities enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants