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] Improve in-site search #17450

Merged
merged 3 commits into from
Sep 17, 2019
Merged

[docs] Improve in-site search #17450

merged 3 commits into from
Sep 17, 2019

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Sep 16, 2019

  • improve visibility of cursor for selected suggestion (big CSS diff is mostly whitespace. I only forced the selected styled of our theme)
  • increase results displayed
  • open suggestions on focus

Ref: #16502

At some point we probably want to use our own combobox implementation. autocomplete.js has some a11y (nvda announces non-descriptive label when browsing through list) as well as UX issues (scroll when browsing should be handled by the package not the app author)

@eps1lon eps1lon added docs Improvements or additions to the documentation accessibility a11y labels Sep 16, 2019
@mui-pr-bot
Copy link

Details of bundle changes.

Comparing: 1a480fe...a2ad77c

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% 0.00% 331,688 331,688 90,532 90,532
@material-ui/core/Paper 0.00% 0.00% 68,659 68,659 20,458 20,458
@material-ui/core/Paper.esm 0.00% 0.00% 62,098 62,098 19,203 19,203
@material-ui/core/Popper 0.00% 0.00% 28,397 28,397 10,159 10,159
@material-ui/core/Textarea 0.00% 0.00% 5,082 5,082 2,132 2,132
@material-ui/core/TrapFocus 0.00% 0.00% 3,766 3,766 1,596 1,596
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,348 16,348 5,818 5,818
@material-ui/core/useMediaQuery 0.00% 0.00% 2,488 2,488 1,050 1,050
@material-ui/lab 0.00% 0.00% 154,749 154,749 46,885 46,885
@material-ui/styles 0.00% 0.00% 51,420 51,420 15,288 15,288
@material-ui/system 0.00% 0.00% 15,581 15,581 4,351 4,351
Button 0.00% 0.00% 78,610 78,610 24,024 24,024
Modal 0.00% 0.00% 14,542 14,542 5,114 5,114
Portal 0.00% 0.00% 2,907 2,907 1,318 1,318
Rating 0.00% 0.00% 69,963 69,963 21,856 21,856
Slider 0.00% 0.00% 75,084 75,084 23,184 23,184
colorManipulator 0.00% 0.00% 3,835 3,835 1,519 1,519
docs.landing 0.00% 0.00% 52,232 52,232 13,768 13,768
docs.main +0.11% 🔺 +0.10% 🔺 597,197 597,827 190,673 190,873
packages/material-ui/build/umd/material-ui.production.min.js 0.00% 0.00% 302,599 302,599 86,969 86,969

Generated by 🚫 dangerJS against a2ad77c

Copy link
Member

@oliviertassinari oliviertassinari left a comment

Choose a reason for hiding this comment

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

Sweet :)!

apiKey: '1d8534f83b9b0cfea8f16498d19fbcab',
indexName: 'material-ui',
inputSelector: '#docsearch-input',
algoliaOptions: {
facetFilters: ['version:master', `language:${userLanguage}`],
hitsPerPage: 40,
Copy link
Member

Choose a reason for hiding this comment

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

Random thought: I'm wondering if it's not too many options 🤔?

Copy link
Member Author

Choose a reason for hiding this comment

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

Why would it be too many?

Copy link
Member

@oliviertassinari oliviertassinari Sep 16, 2019

Choose a reason for hiding this comment

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

The initial thought comes from the tradeoff Algolia took in the default value of this option. Aside from that, I can only guess and make inaccurate reasoning.

We can take the query "modal", let's say we are looking for the API, with 40 results, we have to scroll 10 results, with the default value, we can see it without scrolling.

https://www.smartinsights.com/search-engine-optimisation-seo/seo-analytics/comparison-of-google-clickthrough-rates-by-position/ could be an interesting resource. Seems that most people stop looking after 10 results.

Copy link
Member Author

@eps1lon eps1lon Sep 16, 2019

Choose a reason for hiding this comment

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

We can take the query "modal", let's say we are looking for the API, with 40 results, we have to scroll 10 results, with the default value, we can see it without scrolling.

I don't understand this point. How can something be hidden with less results?

Copy link
Member Author

Choose a reason for hiding this comment

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

Seems that most people stop looking after 10 results.

So lets improve the situation for the people who keep looking.

Copy link
Member

Choose a reason for hiding this comment

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

Agree, if we can help the few without harming the many, it's good to take 👍

},
}));
}),
{ name: 'AppSearch' },
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to provide a name?

Copy link
Member Author

Choose a reason for hiding this comment

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

Better DX. IMO I would add a lint rule against omitting the name. Makes CSS debugging hard.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe we need a Babel plugin that adds it automatically based on the filename 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense Probably based on the var name since this will automatically work for multiple components per file.

@eps1lon eps1lon merged commit 6af6112 into mui:master Sep 17, 2019
@eps1lon eps1lon deleted the docs/better-search branch September 17, 2019 06:54
nrkroeker added a commit to nrkroeker/material-ui that referenced this pull request Sep 18, 2019
* [docs] Batch small changes (mui#17435)

* [docs] Add synonyms for brand icons (mui#17455)

* [docs] Add synonyms for brand icons

* Remove Ic prefixed icons

* [docs] Improve in-site search (mui#17450)

* [ButtonBase] Fix blurry text issue (mui#17453)

* [docs] Revert hits per page change (mui#17458)

* [docs] Fix heading format in CONTRIBUTING.md (mui#17460)

* [Chip] Load the right version of Avatar (mui#17469)

* [TablePagination] Merge root classes properly (mui#17467)

* [Breadcrumbs] Improve API docs (mui#17468)

* [TextField] Handle Chrome autofill (mui#17436)

* [docs] Clarify props spread for ListItem when button flag is set (mui#17466)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility a11y docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants