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

Implement arrow interactions as arrowPress #300

Closed
wants to merge 1 commit into from

Conversation

juanca
Copy link
Collaborator

@juanca juanca commented May 28, 2020

What: Add arrow interactions to the user-event API

Why: Arrow interactions emit key down event, maybe a key press event (I need help on this), and a key up event -- in that particular order. Arrow interactions are crucial in implementing accessibility support for UI widgets. However, there are more than just arrows -- we also need a HOME and END (to name a couple) keys support.

How: Adds an arrowPress function to the module API -- however, perhaps it makes more sense to expose these interactions as a different API (and for better long-term purposes)

Checklist:

  • Documentation
  • Tests
  • Typings
  • Ready to be merged

This is not ready to be merged. As I developed this, I figured I should get more feedback on the API design and testing design. I can follow up on documentation and typing -- though, I will be honest, I don't know how typing are defined in this repository -- and any additional PR checklist/revisions/what-have-yous.

Thank you for your time!

@codecov
Copy link

codecov bot commented May 28, 2020

Codecov Report

Merging #300 into master will increase coverage by 0.02%.
The diff coverage is 92.30%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
+ Coverage   98.90%   98.92%   +0.02%     
==========================================
  Files           1        1              
  Lines         182      186       +4     
  Branches       56       56              
==========================================
+ Hits          180      184       +4     
  Misses          2        2              
Impacted Files Coverage Δ
src/index.js 98.92% <92.30%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5cfbd4...7d1f6e4. Read the comment docs.

@juanca
Copy link
Collaborator Author

juanca commented May 28, 2020

Uh, looks like my IDE removed a bunch of things. 🤔 I didn't see those changes in git add --patch.

EDIT: I would still appreciate some feedback.

@juanca juanca closed this May 28, 2020
@juanca
Copy link
Collaborator Author

juanca commented Jun 17, 2020

Note for future self:

  • Implement generic keydown -- we want to use Home and End keys too! Custom shortcuts should be supported
  • Always fire event on the document.activeElement. That is how a user would interact with a UI.

@nickmccurdy
Copy link
Member

This seems similar to #354, what do you think about it? I started writing tests but you can help out with the implementation if you'd like.

@juanca
Copy link
Collaborator Author

juanca commented Jun 18, 2020

I'm open to collaborating. Let me know the repo#branch combo and I can start digging later today / tomorrow as I balance some work priorities with downtime.

@nickmccurdy
Copy link
Member

Sorry I got distracted and my test doesn't really have any useful assertions (just a snapshot that's currently incorrect), but here's what I started with as an example:

test('{right} on a radio button', () => {
  const {element: button, getEventCalls} = setup(
    <>
      <input type="radio" name="example" defaultChecked />
      <input type="radio" name="example" />
      <input type="radio" name="example" />
    </>,
  )

  userEvent.type(button, '{right}')
})

After this only the second radio should be checked.

@juanca
Copy link
Collaborator Author

juanca commented Jun 20, 2020

I ended up going a different route: using the event snapshot. See #371

Feedback on the PR would be appreciated. Let me know if this is aligned with your thoughts on the user-event API -- I may have done one too many assumptions.

@juanca juanca deleted the arrow-interactions branch June 20, 2020 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants