hub icon indicating copy to clipboard operation
hub copied to clipboard

Auto-focus search form input field

Open jpetazzo opened this issue 3 years ago • 3 comments

This sets the autoFocus attribute on the input field of the search form. It does it only when the search form is displayed on the home page, not on the search result page.

It also sets the autoFocus attribute on the Email field of the LogIn Modal, for two reasons:

  • convenience (field gets automatically focused when trying to log in)
  • perhaps security (we're worried that the login modal might pop up spontaneously, for instance if the user's session expires, and the user might start typing their credentials while the search form is still selected)

Note: I haven't been able to test that latter behavior (I don't know how to trigger the login modal to appear without also interacting with the mouse, which would change the focus anyway...) so I don't know if this is strictly necessary, but the convenience improvement might be useful anyway.

Closes #1675.

jpetazzo avatar Aug 22 '22 16:08 jpetazzo

Hi @jpetazzo

Thanks for the PR! 😊

I've just tested it and there are a couple of details that maybe we could improve:

  • There is a number of modals that can show up automatically in the home page, like the password reset one. When any of them is visible, I think it'd be better to not set the focus on the search bar. One option to achieve this would be to check for the url path to be / and the query string to be empty, as this is not the case then any of the modals should be displayed.

  • On mobile devices, setting the focus on the search bar may open -depending on the browser- the keyboard automatically, which doesn't look very nice and many sites usually avoid. Maybe we could avoid doing this by checking that for the xs and sm breakpoints the focus is not automatically set. What do you think?

Please let me know if you need any help, happy to assist 😊

cynthia-sg avatar Aug 24 '22 11:08 cynthia-sg

Hi @cynthia-sg,

Thanks a lot for the feedback! A couple of questions so that I can get started on the changes - how can I reproduce this for testing? Specifically:

  • When I click on the password reset link, the modal shows up but the URL path doesn't change (it stays localhost:3000); do you know (or where would you recommend me to look at to find out) how to generate an URL that would cause the modal to show up?

  • Do you know any mobile browser that would exhibit the issue, so that I can check if I get it right? (I tried with Brave, Chrome, Firefox Focus on Android, and none of them showed the keyboard😅)

Thank you!

jpetazzo avatar Aug 24 '22 12:08 jpetazzo

No worries 😊

  • When I click on the password reset link, the modal shows up but the URL path doesn't change (it stays localhost:3000); do you know (or where would you recommend me to look at to find out) how to generate an URL that would cause the modal to show up?

The reset link should be something like /reset-password?code=…. However, after the modal is rendered we set it to / by doing a replace. So by the time the autofocus is set for the first time, is should not be / yet and the check should work fine. The autofocus should only work the first time the component is loaded, so replacing the url won't change it.

location
  • Do you know any mobile browser that would exhibit the issue, so that I can check if I get it right? (I tried with Brave, Chrome, Firefox Focus on Android, and none of them showed the keyboard😅)

I've seen the keyboard apearing automatically on Chrome on iOS (iPhone), maybe the behaviour is not the same as in Android (unfortunately I don't have an Android phone at hand to test it right now). In the rest of the mobile browsers I tested (Safari, Firefox and Brave on iOS) the keyboard didn't pop up automatically.

cynthia-sg avatar Aug 24 '22 14:08 cynthia-sg

Hi @jpetazzo

Just in case it helps, maybe you could use the useLocation hook from react-router-dom to do the path and query string checks. To disable the auto focus on small devices, you could use the hook useBreakpointDetect, which is already added to the project.

Let me know if there is anything I can help you with :slightly_smiling_face:

cynthia-sg avatar Nov 10 '22 07:11 cynthia-sg

Hi! :wave: I've just created a new PR (#2721) with your commit plus another one addressing the minor issues I mentioned. Hope you don't mind :innocent:

cynthia-sg avatar Jan 31 '23 09:01 cynthia-sg

Thank you so much for making this happen - and sorry that I wasn't able to put more time into it! 🙏🏻

jpetazzo avatar Feb 01 '23 21:02 jpetazzo