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

Fix: autosuggest breaks search blocks #2219

Merged
merged 4 commits into from
Jun 15, 2021

Conversation

dinhtungdu
Copy link
Contributor

@dinhtungdu dinhtungdu commented May 27, 2021

Description of the Change

Autosuggest changes the frontend markup of the search field to add the suggestion list, while it works for the normal search field, it's breaking the search block styling due to CSS conflict between autosuggest and search block. Because the markup of the search block is predictable, this PR adds a separated behavior for search field in the search blocks that reuses the block markup to add the suggestion list and required class.

Alternate Designs

n/a

Benefits

Make autosuggest work with search block, which is a preparation for full site editing.

Possible Drawbacks

n/a

Verification Process

  1. Enable and configured Autosuggest features.
  2. Create a new page and add a search block.
  3. View the page on the front end, type a keyword, see the suggestions.

Checklist:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my change.
  • All new and existing tests passed.

Applicable Issues

#2211

Changelog Entry

Fixed the styling issue of Autosuggest and search block.

@dinhtungdu
Copy link
Contributor Author

Some screenshots:

The breaking style:

Screenshot at May 27 11-33-50

The style after applying this PR:

image

@Rahmon
Copy link
Contributor

Rahmon commented Jun 1, 2021

Hi @dinhtungdu

When the result has more than one item, the list appears cut and I can see just the first item.

ep-search.mp4

@Rahmon Rahmon assigned dinhtungdu and unassigned Rahmon Jun 1, 2021
@dinhtungdu
Copy link
Contributor Author

@Rahmon Look like you used 2020 for your test. That issue causes by 2020 styling which set overflow: hidden; to the <main> element:
image

It really depends on the theme styling to fix that issue. The issue doesn't occur on Storefront for example:
image

@dinhtungdu dinhtungdu assigned Rahmon and unassigned dinhtungdu Jun 2, 2021
@Rahmon Rahmon requested a review from JakePT June 2, 2021 15:37
@Rahmon
Copy link
Contributor

Rahmon commented Jun 2, 2021

@dinhtungdu got you.

Looks good to me. But I would like to have second thought here so I'm adding @JakePT as a reviewer too since he can contribute with your frontend knowledge.

Copy link
Contributor

@JakePT JakePT left a comment

Choose a reason for hiding this comment

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

@dinhtungdu Just added a small suggestion.

assets/js/autosuggest.js Outdated Show resolved Hide resolved
@dinhtungdu dinhtungdu force-pushed the fix/2211-autosuggest-search-block branch from 6de5091 to 7c150e6 Compare June 3, 2021 06:42
@dinhtungdu
Copy link
Contributor Author

@JakePT Thanks so much for the tip! @Rahmon I force-pushed the branch to clean the history and the diff. This PR is ready to review again.

@Rahmon Rahmon requested a review from JakePT June 7, 2021 18:25
@Rahmon Rahmon merged commit 8523357 into develop Jun 15, 2021
@Rahmon Rahmon deleted the fix/2211-autosuggest-search-block branch June 15, 2021 19:07
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.

4 participants