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

[FEATURE] When clicking on the search icon button, focus the search input #1019

Merged
merged 9 commits into from
Mar 21, 2019
Merged

[FEATURE] When clicking on the search icon button, focus the search input #1019

merged 9 commits into from
Mar 21, 2019

Conversation

dmshm
Copy link
Contributor

@dmshm dmshm commented Mar 13, 2019

Description

Focus the search field when the search button is clicked.

Related Issue

Closes #1018.

Motivation and Context

When clicking the search button icon the field was not focused.

How Has This Been Tested?

The behavior of the item has been tested on the PC as well as on mobile devices.

Screenshots

Screenshot from 2019-03-13 13-57-12
Screenshot from 2019-03-13 13-54-21

Proposed Labels for Change Type/Package

BUG, pkg:venia-concept

Checklist:

  • I have read the CONTRIBUTING document.
  • I have linked an issue to this PR.
  • I have indicated the change type and relevant package(s).
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All CI checks are green (linting, build/deploy, etc).
  • At least one core contributor has approved this PR.

@vercel
Copy link

vercel bot commented Mar 13, 2019

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

@vercel vercel bot temporarily deployed to staging March 13, 2019 12:19 Inactive
@coveralls
Copy link

coveralls commented Mar 13, 2019

Coverage Status

Coverage decreased (-0.02%) to 76.901% when pulling 6835c80 on DmitryShmaliuk:BUG_When_clicking_on_the_search_icon_a_frame_appears_#1018 into 41bff88 on magento-research:develop.

Added 'outline:none' property for all buttons, and removed property from searchTrigger.
@dmshm dmshm changed the title [BUG] When clicking on the search icon, a frame appears #1018 [BUG] When clicking on the button, a frame appears #1018 Mar 13, 2019
@dmshm dmshm changed the title [BUG] When clicking on the button, a frame appears #1018 [BUG] When clicking on the button, a frame appears Mar 13, 2019
@@ -99,6 +99,7 @@ button {
padding: 0;
touch-action: manipulation;
user-select: none;
outline: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

It is considered a bad practice to remove the outline from an active element (http://www.outlinenone.com/).

I would start by asking why the outline on the button feels wrong.

Perhaps, the search button should move the focus to the input when the button is tapped?
Or the style of the outline does not meet the expectations?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@belbiy good remark, thank you. I read the article you provided and I agree with you that the outline should be left on the desktop, but do we need it on mobile devices?

Copy link
Contributor Author

@dmshm dmshm Mar 14, 2019

Choose a reason for hiding this comment

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

It also seems to me that we need to move the focus after we have clicked on the active element. For example: Click on the search element, and after, the focus is transferred to the input field to search.

Or, if you use only "Tab", the items will stand out.

Copy link
Contributor Author

@dmshm dmshm Mar 14, 2019

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

@DmitryShmaliuk, we cant just remove the focus ring for mobile. There's an interesting prototype for input modality that handles the keyboard-mouse input switching.

But in reality, i think that we can improve user experience greatly by doing simple things first:

  1. For search button, move the focus to the input field, so the user does not need to make extra tap. Consider the same behavior for every button – tapping a button can move focus the the area that button controls. If the button opens a shopping cart, for example, move focus inside that modal, etc. This can be a feature of the modal component actually, receiving focus on open, possibly even "trapping" the focus inside itself until closed.

  2. Styling the :focus state of the buttons and other active elements that they look consistent and not "weird" when focused.

@soumya-ashok, what do you think?

Choose a reason for hiding this comment

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

@belbiy Agree with your feedback. I'd just be cautious with choosing the default focus area in a scenario like the cart where there are multiple focusable elements and there aren't traditional form fields in the same way search or checkout has. We should choose a default that causes the least friction for the user in this scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@belbiy, i agree with you, and i will try to implement the your points.

@dmshm dmshm changed the title [BUG] When clicking on the button, a frame appears [FEATURE] When clicking on the button, a frame appears Mar 15, 2019
Implemented focus for input after click on search button.
d.shmaliuk added 2 commits March 15, 2019 12:08
…lemented focus for input after click on search button.
…018' of github.com:DmitryShmaliuk/pwa-studio into BUG_When_clicking_on_the_search_icon_a_frame_appears_#1018
Attempt to specify a problem with 'build-deploy' test.
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

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

👍

@sirugh sirugh changed the title [FEATURE] When clicking on the button, a frame appears [FEATURE] When clicking on the search icon button, focus the search input Mar 21, 2019
@sirugh sirugh merged commit c7a8b32 into magento:develop Mar 21, 2019
@sirugh sirugh removed their assignment Mar 22, 2019
@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label May 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] When clicking on the button, a frame appears
6 participants