Skip to content

Datepicker maintain focus prop #2391

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

Conversation

BenjaminWFox
Copy link
Contributor

@BenjaminWFox BenjaminWFox commented Nov 2, 2019

Fixes #2285

Additional description

Currently in the datepicker, interaction with the Input element moves focus to the Calendar. This PR adds an optional prop canInputMaintainFocus to the Datepicker which, when true, allows clicking & interacting with the input element without losing focus.

I'm unsure if solving this by adding an additional prop is acceptable, but there hadn't been any movement on this issue in the last month so I thought I would try. This could also be adapted into the new default behavior pretty easily.

I believe that with canInputMaintainFocus set to true all of the pattern bullet points are satisfied.

I added a story, but have not added a browser-test explicitly for this as I had trouble simulating the focus event. If that would be a requirement please let me know.


CONTRIBUTOR checklist (do not remove)

Please complete for every pull request

  • First-time contributors should sign the Contributor License Agreement. It's a fancy way of saying that you are giving away your contribution to this project. If you haven't before, wait a few minutes and a bot will comment on this pull request with instructions.
  • npm run lint:fix has been run and linting passes.
  • Mocha, Jest (Storyshots), and components/component-docs.json CI tests pass (npm test).
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.

REVIEWER checklist (do not remove)

  • TravisCI tests pass. This includes linting, Mocha, Jest, Storyshots, and components/component-docs.json tests.
  • Tests have been added for new props to prevent regressions in the future. See readme.
  • Review the appropriate Storybook stories. Open http://localhost:9001/.
  • The Accessibility panel of each Storybook story has 0 violations (aXe). Open http://localhost:9001/.
  • Review tests are passing in the browser. Open http://localhost:8001/.
  • Review markup conforms to SLDS by looking at DOM snapshot strings.
Required only if there are markup / UX changes
  • Add year-first date and commit SHA to last-slds-markup-review in package.json and push.
  • Request a review of the deployed Heroku app by the Salesforce UX Accessibility Team.
  • Add year-first review date, and commit SHA, last-accessibility-review, to package.json and push.
  • While the contributor's branch is checked out, run npm run local-update within locally cloned site repo to confirm the site will function correctly at the next release.

@welcome
Copy link

welcome bot commented Nov 2, 2019

Thanks for opening this pull request! 💯

This is a community-driven project, and we can't do it without your participation. Please check out our contributing guidelines and review the Contributor Checklist if you haven't already, to make sure everything is squared away. TravisCI will take about 10 minutes to run through the same items that are on the Contributor checklist with a pass/fail check below. Please fix any issues that cause TravisCI to fail or ask for clarification--we try, but sometimes the errors can be unclear.
A maintainer will try to respond within 7 days. If you haven’t heard anything by then, please bump this thread. To ensure codebase quality, large code line changes may take more than 2 weeks to review, but may take longer depending on the number of pull requests in the queue. Feel free to ask for a status update at any time--you won't be bothering anyone.
Once feedback has been given, please reply to the feedback giver once the feedback on been addressed, so that they can continue the review.
If you need a release while you are waiting for a code review, you can publish a built tag to your own fork. See directions in the release README.

@salesforce-cla
Copy link

salesforce-cla bot commented Nov 2, 2019

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Ben Fox <b***@s***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Salesforce.com Contributor License Agreement and this Pull Request will be revalidated.

@salesforce-cla
Copy link

salesforce-cla bot commented Nov 2, 2019

Thanks for the contribution! Before we can merge this, we need @BenjaminWFox to sign the Salesforce.com Contributor License Agreement.

@BenjaminWFox
Copy link
Contributor Author

BenjaminWFox commented Nov 2, 2019

Sorry, this appears to have some commits from something I playing with some time ago. I will see if I can remove them from this PR, otherwise I will close this and submit a new PR.

ETA: junk commits removed.

@BenjaminWFox BenjaminWFox force-pushed the datepicker-maintain-focus-prop branch from 3ccce47 to a35430f Compare November 2, 2019 21:07
@interactivellama interactivellama self-requested a review November 16, 2019 04:38
@interactivellama
Copy link
Contributor

interactivellama commented Nov 16, 2019

Thank you so much for carrying this forward! @BenjaminWFox

Apologies for the delay on the feedback. I think this is good pattern. It can be the default without the prop. You can remove the new prop.

The primary issue I see is that explicitly opening the menu with the right side icon should still focus on the menu day. It is currently not. The focus is staying on the icon

Something like a menuOpenedByIcon prop on CalendarWrapper would work and only be true when the icon is clicked, but not when the rest of the input is clicked into.

And please run npm run lint:fix to lint code. https://travis-ci.org/salesforce/design-system-react/jobs/606556443#L7438

Here is a screenshot:
Screen Shot 2019-11-15 at 9 46 39 PM

Copy link
Contributor

@interactivellama interactivellama left a comment

Choose a reason for hiding this comment

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

Please see request left in previous comment.

@BenjaminWFox BenjaminWFox force-pushed the datepicker-maintain-focus-prop branch from 3facc4d to c4ddb70 Compare November 21, 2019 22:12
@BenjaminWFox BenjaminWFox force-pushed the datepicker-maintain-focus-prop branch from c4ddb70 to c80a06e Compare November 21, 2019 22:42
@BenjaminWFox
Copy link
Contributor Author

@interactivellama This is updated per your suggestions, let me know if any further edits are needed, thanks.

@interactivellama interactivellama self-requested a review November 25, 2019 22:59
@interactivellama
Copy link
Contributor

I pulled down and viewed in Storybook and the UX pattern is looking great! Give me a few days to look over the code and we should be good.

@interactivellama interactivellama merged commit 58d3ef6 into salesforce:master Dec 3, 2019
@welcome
Copy link

welcome bot commented Dec 3, 2019

Congrats on merging your first pull request to Design System React! 🎉
If you have a moment, please fill out this five question survey, we would appreciate it.
On behalf of Salesforce's customers, partners, product specialists and employees, we would like offer sincere thanks and appreciation for helping make our user experience better. We look forward to working with you more in the future.
This definitely calls for a high five! Alt High Five

@interactivellama
Copy link
Contributor

Thank you @BenjaminWFox for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Datepicker: Unable to enter date manually
2 participants