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 accessibility of FreeTube input elements #2709

Merged

Conversation

ChunkyProgrammer
Copy link
Member

@ChunkyProgrammer ChunkyProgrammer commented Oct 11, 2022

Improve accessibility of FreeTube input elements

Pull Request Type

  • Bugfix

Related issue

Implements part of #1525 (removes jquery)
#693

Description

Makes generic ft-(input) items more accessible. Future PRs will tackle more components.

Testing

FtDropdown:

  • Find a Playlist
  • Tab to the "Share" bar (clicking on the description then clicking tab might be easier)
  • Press enter on the "Share"
  • Press enter on copy link

FtNotificationBanner:

  • modify version in package.json to a previous version (0.16.0)(verify that check for app updates is enabled in the setting as well)
  • Tab to notification banner
  • Press enter
  • (Repeat but instead of pressing enter, tab to the close button then press enter)

FtPrompt:

  • go to data settings
  • tab to "Export Subscriptions"
  • press enter
  • click tab then press enter

FtIconButon

  • navigate to a video
  • tab to the share button
  • press enter
  • tab to different options in list
  • press enter

FtSelect

  • go to settings
  • go to download settings
  • tab to "Download Behavior"
  • press arrow keys up/down
  • see that value changes
  • press enter
  • list of values opens up

Desktop

  • OS: Windows
  • OS Version: 11
  • FreeTube version: 0.17.2

Additional context

Co-Authored-By: Jason 84899178+jasonhenriquez@users.noreply.github.com

Co-Authored-By: Jason <84899178+jasonhenriquez@users.noreply.github.com>
@github-actions github-actions bot added PR: dependencies Pull requests that update a dependency file PR: waiting for review For PRs that are complete, tested, and ready for review labels Oct 11, 2022
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) October 11, 2022 04:43
Copy link
Member Author

@ChunkyProgrammer ChunkyProgrammer left a comment

Choose a reason for hiding this comment

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

Some small changes i need to make

src/renderer/components/ft-prompt/ft-prompt.js Outdated Show resolved Hide resolved
@PikachuEXE
Copy link
Collaborator

Can I wave white flag so many things to test ._.

PikachuEXE
PikachuEXE previously approved these changes Oct 11, 2022
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Tested

  • FtDropdown
  • FtPrompt - however if I press tab "export YT(csv)" is highlighted, is it expected
  • FtIconButon
  • FtSelect

Not tested (lazy ._.)

  • FtNotificationBanner

@ChunkyProgrammer
Copy link
Member Author

ChunkyProgrammer commented Oct 11, 2022

  • FtPrompt - however if I press tab "export YT(csv)" is highlighted, is it expected

Fixed, made first focus visible

@PikachuEXE
Copy link
Collaborator

Tested updated FtPrompt

How do I escape from Select Export Type with keyboard only?

@ChunkyProgrammer
Copy link
Member Author

Tested updated FtPrompt

How do I escape from Select Export Type with keyboard only?

Just added a close button

PikachuEXE
PikachuEXE previously approved these changes Oct 12, 2022
src/renderer/components/ft-prompt/ft-prompt.vue Outdated Show resolved Hide resolved
src/renderer/components/ft-prompt/ft-prompt.js Outdated Show resolved Hide resolved
src/renderer/components/ft-prompt/ft-prompt.vue Outdated Show resolved Hide resolved
@absidue
Copy link
Member

absidue commented Oct 12, 2022

Tested updated FtPrompt
How do I escape from Select Export Type with keyboard only?

Just added a close button

Would probably be good to make the Escape key close it too (add an event listener to the document in mounted and remove it in beforeDestroy).

ChunkyProgrammer and others added 2 commits October 12, 2022 16:35
Co-authored-by: absidue <48293849+absidue@users.noreply.github.com>
@ChunkyProgrammer
Copy link
Member Author

@absidue i believe that I addressed your concerns

PikachuEXE
PikachuEXE previously approved these changes Oct 13, 2022
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

Escape!!!

@efb4f5ff-1298-471a-8973-3d47447115dc

Hi @RastislavKish Is it possible for u to help out with testing this PR? This PR only implements a part of #1525 but we want to make sure the things implemented in this PR behaves the same for u as #1525.

@efb4f5ff-1298-471a-8973-3d47447115dc

i think this closes #1580

@ChunkyProgrammer ChunkyProgrammer added the PR: waiting for review For PRs that are complete, tested, and ready for review label Nov 16, 2022
PikachuEXE
PikachuEXE previously approved these changes Nov 17, 2022
@PikachuEXE
Copy link
Collaborator

What stuff is changed that should be tested ._.

@ChunkyProgrammer
Copy link
Member Author

ChunkyProgrammer commented Dec 1, 2022

What stuff is changed that should be tested ._.

It's a pretty small change, just check that playlist sharing options work as expected . You should test it with a language like Korean, Faroese, Chinese, Japanase, Arabic or Hebrew

PikachuEXE
PikachuEXE previously approved these changes Dec 1, 2022
Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

FtDropdown re-tested with zh-tw

@github-actions github-actions bot added PR: merge conflicts / rebase needed and removed PR: waiting for review For PRs that are complete, tested, and ready for review labels Dec 7, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

Conflicts have been resolved. A maintainer will review the pull request shortly.

PikachuEXE
PikachuEXE previously approved these changes Dec 8, 2022
@PikachuEXE
Copy link
Collaborator

@efb4f5ff-1298-471a-8973-3d47447115dc @absidue
Let's not make this PR hanging too long?

@absidue absidue added the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 10, 2022
@FreeTubeBot FreeTubeBot merged commit 45953e3 into FreeTubeApp:development Dec 11, 2022
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Dec 11, 2022
@ChunkyProgrammer ChunkyProgrammer deleted the accessibility-inputs branch December 11, 2022 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants