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

[docs] Upgrade Algolia search UI #26397

Closed
wants to merge 12 commits into from
Closed

[docs] Upgrade Algolia search UI #26397

wants to merge 12 commits into from

Conversation

arpitBhalla
Copy link
Contributor

@arpitBhalla arpitBhalla commented May 20, 2021

@arpitBhalla arpitBhalla marked this pull request as draft May 20, 2021 16:32
@mui-pr-bot
Copy link

mui-pr-bot commented May 20, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 83acbf3

@arpitBhalla arpitBhalla marked this pull request as ready for review May 21, 2021 03:53
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Not sure what happened with your lockfile. Try reverting that change (but keep the change to the package.json) and then run yarn install again. But make sure your run yarn install from the workspace root not ./docs

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label May 21, 2021
@oliviertassinari oliviertassinari changed the title [website] New Algolia search [docs] Upgrade Algolia search UI May 21, 2021
@oliviertassinari
Copy link
Member

Having a quick look at the preview URL, I can notice that:

  • the background-color is broken
  • the colors aren't working when in dark mode

Regarding the button that triggers the modal, we will likely need to rework the UI. I would propose we keep the previous style (both for the docs, and the website, so two different versions)

@siriwatknp
Copy link
Member

siriwatknp commented May 24, 2021

@oliviertassinari styling is done but I think the search result is not good.

Light mode

image

Dark mode

image

  • Color contrast checked

@oliviertassinari
Copy link
Member

oliviertassinari commented May 24, 2021

@siriwatknp Great, thanks for taking the lead on this one. This can be a great side project when you are waiting for feedback on one of the breaking changes.

What I could find, so far:

  1. The instructions at the bottom seem overzealous, over-explaining how the UI work. What do you think about hiding them?

Screenshot 2021-05-24 at 18 35 17

  1. The vertical alignment of the search seems off, compared to where the input is:

Screenshot 2021-05-24 at 18 36 51

  1. The link font-weight and color seems off:

Screenshot 2021-05-24 at 18 38 32

  1. When we trigger the link, a full-page navigation is executed, not a client-side navigation with Next.js (this is harming the UX)

  2. By default, when we open the modal, there is a strange scroll: visible even if there is no scroll bar. It doesn't looks great.

Screenshot 2021-05-24 at 18 40 55

  1. The font-variant of "No recent searches" is off, maybe it should be theme.typography.body2

Screenshot 2021-05-24 at 18 43 49

  1. The results seems completely off. I suspect we need to ask Algolia to configure our index. Compare the same query:

Screenshot 2021-05-24 at 18 47 22

Screenshot 2021-05-24 at 18 47 29

If it's true, then we might need to have two accounts, one for v4, and one for v5.

  1. Sebastian started to raise issues in [docs-infra] Improve Algolia internal search results on mui.com #16502
  2. "Recent" is empty, is it expected? It doesn't seem to manage to save the history.
  3. Regarding the integration with the rest of the website. We will need to have it in https://deploy-preview-26397--material-ui.netlify.app/branding/about/ too :). We will need to customize the look of the input, probably best to write our own input here.

when I search TextField, the components should comes first, not the API?

Agree

the Hit path should be removed, I think it is too much

What do you mean by "Hit path"?

@oliviertassinari oliviertassinari added the priority: important This change can make a difference label Jun 5, 2021
@arpitBhalla
Copy link
Contributor Author

@siriwatknp Please review the changes,

@oliviertassinari
Copy link
Member

oliviertassinari commented Jun 13, 2021

@arpitBhalla thanks for the udpate. I had a look at the changes 7 days ago. We are off from the target, so much so that, it felt like a WIP, I didn't take the time to comment as I felt most of the feedback of #26397 (comment) were not taken into account yet (I see 2/10 that might have been fixed). Note that these ten items are unlikely to be enough. I could already found a new obvious one, so I assume a deeper review will uncover improvement opportunities. I hope it helps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to the documentation priority: important This change can make a difference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Change Algolia search implementation
5 participants