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: autocomplete to alpha.44 #227

Merged
merged 5 commits into from
Mar 4, 2021
Merged

fix: autocomplete to alpha.44 #227

merged 5 commits into from
Mar 4, 2021

Conversation

sbellone
Copy link
Collaborator

@sbellone sbellone commented Mar 2, 2021

Changes

  • Updated to alpha.44
  • Re-imported updated autocomplete-theme-classic and updated our overriding CSS:
    • Some CSS classes have been renamed (Touch -> Detached), updated or removed (.aa-Panel--desktop)
  • Fixed detached mode CSS (background color was not respecting the theme)
  • Exposed detachedMediaQuery to let user control the detached mode (https://algolia-autocomplete.netlify.app/docs/detached-mode/#what-is-detached)

How to test

  • yarn dev
  • Reduce your windows size to trigger detached mode

@sbellone sbellone self-assigned this Mar 2, 2021
@bodinsamuel
Copy link
Contributor

This is a duplicate of #220
Also I'm awaiting this fix algolia/autocomplete@5991e77 to be released

@sbellone
Copy link
Collaborator Author

sbellone commented Mar 2, 2021

I hadn't seen it, but:

  • We don't want to "revert" the touch UI, it's better to keep it and expose a parameter
  • _highlightResult is not part of our record shema
  • I've taken the time to update the whole CSS and make the detached UI work correctly

Let's release this one to give a workaround to people running the UI on Gatsby?

Copy link
Contributor

@bodinsamuel bodinsamuel left a comment

Choose a reason for hiding this comment

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

  • The clear button has not the correct color when hovered
  • The detached input has not the same size

frontend/src/autocomplete-theme-classic.scss Outdated Show resolved Hide resolved
public/index.html Outdated Show resolved Hide resolved
@sbellone
Copy link
Collaborator Author

sbellone commented Mar 2, 2021

The clear button has not the correct color when hovered

What is the correct color? There is not hover on a touch device anyway.

The detached input has not the same size

Yes it's a design choice, to look more like a button I imagine: https://github.com/algolia/autocomplete/blob/next/packages/autocomplete-theme-classic/src/theme.scss#L482

@bodinsamuel
Copy link
Contributor

What is the correct color? There is not hover on a touch device anyway.

Well the bug is still there anyway, correct color could be white idk

Yes it's a design choice, to look more like a button I imagine:

it's up to us to decide our own design, and I think it's better if both type of input are the same height no?

@sbellone
Copy link
Collaborator Author

sbellone commented Mar 3, 2021

Well the bug is still there anyway, correct color could be white idk

Ok I thought we were talking the Cancel button, this bug was already here and I hadn't retested this part.

For the input height, we've already shipped it like that last time and I think it makes sense to have it look like a button since it opens a fullscreen search. One less CSS property to override also.

@sbellone sbellone merged commit 9a467eb into master Mar 4, 2021
@sbellone sbellone deleted the fix/autocomplete-alpha-44 branch March 4, 2021 10:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants