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.

Sign up for GitHub

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

Jump to bottom

Accessibility, keyboard support #63

Merged
merged 20 commits into from
Sep 25, 2020
Merged

Accessibility, keyboard support #63

merged 20 commits into from
Sep 25, 2020

Conversation

omgovich
Copy link
Owner

Closes #57

@omgovich omgovich self-assigned this Sep 23, 2020
@github-actions
Copy link

github-actions bot commented Sep 23, 2020 •
edited
Loading

Size Change: +131 B (3%)

Total Size: 3.83 kB

Filename Size Change
./dist/index.css 698 B +1 B
dist/index.module.js 3.13 kB +130 B (4%)

compressed-size-action

@omgovich omgovich mentioned this pull request Sep 24, 2020
Closed
@omgovich
Copy link
Owner Author

omgovich commented Sep 24, 2020 •
edited
Loading

Had to apply a few tricks to keep all pickers inside 2 KB limit.

@omgovich omgovich marked this pull request as ready for review September 24, 2020 15:05
@omgovich omgovich requested a review from molefrog September 24, 2020 15:12
@omgovich
Copy link
Owner Author

@molefrog @rschristian Hey guys! Could I ask you to take a look?

@omgovich
Copy link
Owner Author

HexColorPicker size:

1.51 → 1.86 KB

I added a lot of new code, so I think it's a pretty good result.

molefrog
molefrog reviewed Sep 24, 2020
View reviewed changes
src/utils/limit.ts Outdated Show resolved Hide resolved
@rschristian
Copy link
Contributor

Just going by the changes (on my phone, can't test) it looks pretty good.

@omgovich
Copy link
Owner Author

Added few specs to test keyboard support.

rschristian
rschristian reviewed Sep 24, 2020
View reviewed changes
src/utils/convert.ts Outdated Show resolved Hide resolved
src/utils/convert.ts Outdated Show resolved Hide resolved
src/utils/convert.ts Outdated Show resolved Hide resolved
src/utils/clamp.ts Show resolved Hide resolved
molefrog
molefrog approved these changes Sep 25, 2020
View reviewed changes
Copy link
Collaborator

@molefrog molefrog left a comment

Choose a reason for hiding this comment

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

Great work!

@omgovich
Copy link
Owner Author

Thanks, @molefrog @rschristian!

@omgovich omgovich merged commit 95b067d into master Sep 25, 2020
@omgovich omgovich deleted the feature/a11y branch September 25, 2020 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rschristian rschristian rschristian left review comments

@molefrog molefrog molefrog approved these changes

Assignees

@omgovich omgovich

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement accessibility based on ARIA practices
3 participants
@omgovich @rschristian @molefrog