Open chebizarro opened 3 weeks ago
@chebizarro thank you for the PR!
I think this is a huge change and we need to split it up, and focus on one thing per PR and make sure everything works.
Currently I think the most important thing is standard keyboard accessibility as normal keyboard power-users would find this useful.
I ran your PR and opened the dev mode (yarn dev
)
To more easily test this, I would suggest we add a new html file just with one button so we can easily test there.
button.html
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>Vite + Lit + TS</title>
<style>
body {
font-family: Arial, Helvetica, sans-serif;
margin: 0px;
@media (prefers-color-scheme: dark) {
background-color: black;
color: white;
}
}
.container {
padding: 20px;
max-width: 448px;
}
</style>
<script type="module">
import '../../src/index.ts';
</script>
</head>
<body>
<div class="container">
<bc-button></bc-button>
</div>
</body>
</html>
If I load the page and press tab once this is what I see (incorrect item focused):
If I then open the modal I would expect the first connector to be focused by default. And if I tab forward once, it should select the next connector. If I tab backwards once I would expect it to go to the x
and then ?
if I tab backward again. There is a lot of tab and focus issues in this modal.
Do you think we could address this as a first step and maybe open a new PR for this alone?
Once I connect I think it's strange that it focuses on the currency by default. I guess this would not be such an issue if https://github.com/getAlby/bitcoin-connect/issues/100 is implemented (actually, I will see if I can get a bounty created for this issue too).
I refactored the branch and removed the aria
attributes as they aren't necessary for keyboard navigation. I had misinterpreted the goal of this issue as adding accessibility features and not just enhanced keyboard navigation, so apologies for the confusion. This refactoring has resulted in far fewer changes across a much smaller number of files and is much more intelligible.
This pull request aims to address issue #182 by improving accessibility for users by enabling keyboard navigation and screen reader feedback.
The majority of the changes are simple additions of
aria
tags to the rendered html as well as extra handlers for keyboard navigation.These changes will need more thorough testing as there is no simple way to test each component in isolation that I can see.