-
Notifications
You must be signed in to change notification settings - Fork 862
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
Makes twitter tip actions keyboard accessible #3253
Conversation
28cb81e
to
503f316
Compare
@@ -275,6 +277,21 @@ document.addEventListener('visibilitychange', function () { | |||
} | |||
}) | |||
|
|||
// Listens for enter key when Tip action is tab-highlighted | |||
document.addEventListener('keydown', function (e) { | |||
if (e.key === 'Enter') { |
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.
RIP KeyboardEvent.keyCode
👻
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.
This is great! LGTM
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.
please check CI
ERROR in [at-loader] ./components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts:284:26
22:47:26 TS2531: Object is possibly 'null'.
22:47:26
22:47:26 ERROR in [at-loader] ./components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts:287:25
22:47:26 TS2531: Object is possibly 'null'.
22:47:26
22:47:26 ERROR in [at-loader] ./components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts:287:25
22:47:26 TS2531: Object is possibly 'null'.
22:47:26
22:47:26 ERROR in [at-loader] ./components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts:289:19
22:47:26 TS2339: Property 'click' does not exist on type 'Element'.
3ae4059
to
5acb7ac
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.
This will need to work on space (as well as enter) enter, like the other twitter actions. I wonder if just making it a element will have the same effect without this hack, and then also gain the other built-in accessibility features? If not possible, then please at least add support for space unless you see a reason not to.
@@ -275,6 +277,33 @@ document.addEventListener('visibilitychange', function () { | |||
} | |||
}) | |||
|
|||
// Listens for enter key when Tip action is tab-highlighted | |||
document.addEventListener('keydown', function (e) { | |||
if (e.key !== 'Enter' || e.code !== 'Space') { |
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.
KeyboardEvent.key
equals a single blank space " "
for space bar presses, using the code
property for readability
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.
I think you could do this without listening to every single keydown event on the document and could just add the event listener directly to each tip button.
function onTipButtonActivate (...) { ... }
components/brave_rewards/resources/extension/brave_rewards/content_scripts/twitter.ts
Outdated
Show resolved
Hide resolved
CI failure unrelated, version mismatch |
Fixes: brave/brave-browser#5791
Note: It wasn't mentioned in the document, but tab selecting the tip actions didn't work at all, so this PR fixes that as well.
Submitter Checklist:
npm run lint
)git rebase master
(if needed).git rebase -i
to squash commits (if needed).Test Plan:
Test plan is specified in issue
Reviewer Checklist:
After-merge Checklist:
changes has landed on.