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

add attributes aria-label= and title= to Filters & View Modes buttons #5133

Merged
merged 2 commits into from
Oct 16, 2023

Conversation

privatemaker
Copy link
Contributor

@privatemaker privatemaker commented Sep 19, 2023

Summary

The two middle buttons used to toggle filter and show/hide cards

deck-ux-missing-aria-buttons

I added a couple aria-label="..." and title="..." attributes to the components and it resolved the issue.

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included
  • Documentation (manuals or wiki) has been updated or is not required

@privatemaker
Copy link
Contributor Author

I do not know why the eslint is failing in the CI, but here is the log of where the tests fail.

$ make test

...
OK (25 tests, 58 assertions)
cd tests/integration && ./run.sh
Installing dependencies from lock file (including require-dev)
Verifying lock file contents can be installed on current platform.
Nothing to install, update or remove
Generating autoload files

47 packages you are using are looking for funding.
Use the `composer fund` command to find out more!
Nextcloud instance needs to be installed
make: *** [Makefile:67: test-integration] Error 1

The front-end test fails with:

$ npm run test

> [email protected] test
> jest

● Validation Error:

  Module <rootDir>/node_modules/vue-jest in the transform option was not found.
         <rootDir> is: /var/www/nextcloud/apps/deck

  Configuration Documentation:
  https://jestjs.io/docs/configuration

However, that test fails on a fresh clone from HEAD so that has nothing to do with my change. My change does successfully compile with npm run dev | build

@juliusknorr juliusknorr merged commit 6743368 into nextcloud:main Oct 16, 2023
11 of 12 checks passed
@privatemaker
Copy link
Contributor Author

@juliushaertl sorry I was away to fix the eslint thing. Thanks for merging 😄

@privatemaker privatemaker deleted the fix/aria-title-to-buttons branch October 20, 2023 14:48
@juliusknorr
Copy link
Member

No worries, thanks for the contribution :)

Copy link
Contributor

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants