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

Improve Keyboard UX #311

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Improve Keyboard UX #311

wants to merge 2 commits into from

Conversation

froston
Copy link

@froston froston commented Oct 23, 2017 •
edited by benjycui

Datepicker

  • Should accept focus (tabindex)
  • Needs outline when focused
  • Should immediately allow for typing when opened
  • Should close on blur
  • Should close on Esc

Edit: close #310

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 80.396% when pulling b163b77 on froston:master into 02b9e13 on react-component:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.1%) to 80.396% when pulling 8b85cd6 on froston:master into 02b9e13 on react-component:master.

benjycui
benjycui reviewed Oct 25, 2017
View reviewed changes
src/Calendar.jsx
@@ -247,6 +252,7 @@ const Calendar = createReactClass({

const dateInputElement = props.showDateInput ? (
<DateInput
dateInputRef={props.dateInputRef}
Copy link
Member

@benjycui benjycui Oct 25, 2017 •
edited

Choose a reason for hiding this comment

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

What is the use case for this new API?

benjycui
benjycui reviewed Oct 25, 2017
View reviewed changes
src/Picker.jsx
if (!this.props.calendar.props.showDateInput) {
this.focusTimeout = setTimeout(this.focusCalendar, 0, this);
} else {
this.focusTimeout = setTimeout(this.focusDateInput, 0, this);
Copy link
Member

Choose a reason for hiding this comment

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

If we focus directly in date input, then users cannot change date with up down and so on.

@benjycui
Copy link
Member

I am consulting our UX designer for the suggested keyboard experience for rc-calendar.

Maybe you can try rc-collapse first? For we had found an online demo to show the best practice ant-design/ant-design#6017 , and it is simple enough to start.

@froston
Copy link
Author

froston commented Oct 25, 2017

Ok thanks!
I was just following this antd issue. It was accepted with PR welcome.
The only thing which is missing is Should open when focused. I could make it working in all the browsers but IE.
I think it somehow makes sense: if you have input, focus it; if you don't, focus the calendar. And if user wants to navigate thru the calendar anyway, can focus it by mouse and then continue with keyboard.

@benjycui
Copy link
Member

Actually, the keyboard UX of rc-calendar is extremely complex. Hope that you can try other components first, and I think rc-collapse is a better choice.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjycui benjycui benjycui left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve Keyboard UX
3 participants
@froston @coveralls @benjycui