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] Make it easier to find material icons #16956

Merged
merged 5 commits into from
Aug 14, 2019

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Aug 9, 2019

I have invested 2 hours in the early prototype. I want to see if our Netlify infrastructure can handle the 5k icons and the performance in production mode. The build is 60s slower comparing two, to refine. Client-side perf are OK-ish, need to reduce the debounce.

https://deploy-preview-16956--material-ui.netlify.com/components/material-icons/

The problem

  • Reach this level of experience as a minimum baseline: https://ant.design/components/icon/.
  • People have to know how to convert the material.io snack case name into our import structure
  • People have no icon context preview as font-awesome does. I think that we could display them in the most common cases: button, tabs.
  • People search icons on material.io and don't find them (no synonyms).
  • People search icons in Aglolia and don't find them.

TODO

  • Do the icon detail, something similar to https://fontawesome.com/icons/500px?style=brands
  • Add synonyms using the existing search patterns on Algolia
  • See how we could make the page searchable with Algolia
  • Update /components/icon to link the new page, and better document font awesome

Future

  • Use the GA report to add missing synonyms.
  • Support in the search typos
  • List mdi-material-ui too

Capture d’écran 2019-08-09 à 21 34 27

Closes #14984

@oliviertassinari oliviertassinari added the docs Improvements or additions to the documentation label Aug 9, 2019
@oliviertassinari oliviertassinari force-pushed the icon-search branch 2 times, most recently from 36c9319 to 51451e5 Compare August 9, 2019 19:43
@mui-pr-bot
Copy link

mui-pr-bot commented Aug 9, 2019

Details of bundle changes.

Comparing: 7d01949...f8b658e

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% 0.00% 328,898 328,898 89,914 89,914
@material-ui/core/Paper 0.00% 0.00% 68,684 68,684 20,476 20,476
@material-ui/core/Paper.esm 0.00% 0.00% 62,058 62,058 19,206 19,206
@material-ui/core/Popper 0.00% 0.00% 28,468 28,468 10,177 10,177
@material-ui/core/Textarea 0.00% 0.00% 5,094 5,094 2,136 2,136
@material-ui/core/TrapFocus 0.00% 0.00% 3,834 3,834 1,614 1,614
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,386 16,386 5,826 5,826
@material-ui/core/useMediaQuery 0.00% 0.00% 2,541 2,541 1,059 1,059
@material-ui/lab 0.00% 0.00% 152,999 152,999 46,659 46,659
@material-ui/styles 0.00% 0.00% 51,401 51,401 15,290 15,290
@material-ui/system 0.00% 0.00% 15,658 15,658 4,361 4,361
Button 0.00% 0.00% 78,687 78,687 24,044 24,044
Modal 0.00% 0.00% 14,346 14,346 5,011 5,011
Portal 0.00% 0.00% 2,907 2,907 1,318 1,318
Rating 0.00% 0.00% 70,047 70,047 21,881 21,881
Slider 0.00% 0.00% 74,362 74,362 23,056 23,056
colorManipulator 0.00% 0.00% 3,904 3,904 1,543 1,543
docs.landing +0.14% 🔺 +0.04% 🔺 51,846 51,920 13,780 13,786
docs.main +0.08% 🔺 +0.15% 🔺 590,557 591,000 188,449 188,735
packages/material-ui/build/umd/material-ui.production.min.js 0.00% 0.00% 299,897 299,897 86,270 86,270

Generated by 🚫 dangerJS against f8b658e

@mbrookes
Copy link
Member

mbrookes commented Aug 9, 2019

Great start!

The build is 60s slower comparing two

We could build it as a separate site, and use a Netlify path rewrite. The icons don't change very often, so no need to build this page every time.

What do you think about using a radio style input for the "theme" selection (the same as ant and the officials icons page)? I find switching between styles makes it easier to compare them. Being able to select none also seems odd. :)

The preview list of icons should ideally show the icon name (again, same as ant and the official icon docs, as well as font awesome, and semantic ui FA, and...). Perhaps we should show the import name rather than the snake-cased ones?

@oliviertassinari oliviertassinari force-pushed the icon-search branch 6 times, most recently from c1e64f6 to 30948f5 Compare August 10, 2019 13:24
@oliviertassinari
Copy link
Member Author

We could build it as a separate site, and use a Netlify path rewrite. The icons don't change very often, so no need to build this page every time.

I think that a good option is to wait to see if people are using the page before investing time in speeding up the Netlify build (because otherwise, we could revert this PR). On this time build concern, I have run a test, not importing the API markdowns saves 3 minutes. This is strange.

What do you think about using a radio-style input for the "theme" selection (the same as ant and the officials' icons page)? I find switching between styles makes it easier to compare them. Being able to select none also seems odd. :)

I have wondered about it. I find it interesting to display both as you might use different variants in the app. For instance, you might use the filled for the selected state and outlined when unselected. You probably want to see both.

The preview list of icons should ideally show the icon name

I have wondered about this one. I like how ionicons.com doesn't display anything. I think that it helps to pick an icon without been biased by the label. But I don't think strongly. Given your feedback and the fact that font awesome do it, I'm changing it.

@oliviertassinari oliviertassinari force-pushed the icon-search branch 2 times, most recently from 262adaa to 9a4e2c3 Compare August 10, 2019 15:37
@oliviertassinari oliviertassinari marked this pull request as ready for review August 10, 2019 15:38
@oliviertassinari oliviertassinari force-pushed the icon-search branch 2 times, most recently from 5044ce3 to 777e1a6 Compare August 10, 2019 16:39
@joshwooding
Copy link
Member

LGTM! I'm excited to use it :D

@mbrookes
Copy link
Member

mbrookes commented Aug 13, 2019

you might use the filled for the selected state and outlined when unselected. You probably want to see both.

In that case you probably want to toggle between them. Let's go with radios. Overall the checkboxes have poor UX.

Should the dialog be bigger? (truncated import path)

image

There's a stray keyboard focus indicator after closing the dialog (Chrome), and some of the names are truncated (more whitespace needed?):

image

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 14, 2019

In that case you probably want to toggle between them. Let's go with radios. Overall the checkboxes have poor UX.

I think that the problem solved with the display of multiple variants is that it makes it easier to compare them, to pick the one you like most. At least, it has been my use case recently with https://oblador.github.io/react-native-vector-icons/. I'm wondering why Fontawesome uses this checkbox approach.

Should the dialog be bigger? (truncated import path)

I have tried with a bigger dialog, I felt it was creating too much distraction (a larger surface to spend attention on). I have disabled the overflow on the import markdown so we don't see an ugly scrollbar. As soon as you click the area, we select the whole thing. In a sense, you don't need to see everything. Maybe enabling the scroll would be a better tradeoff?

There's a stray keyboard focus indicator after closing the dialog

Yes, it's meant to so that people remember on which icon they clicked. It then easier to move to the next one.

some of the names are truncated (more whitespace needed?):

Ah nice, thanks! I wanted to add an overflow ellipsis, but forgot about it.

@mbrookes
Copy link
Member

I'm wondering why Fontawesome uses this checkbox approach.

Or why Google and Ant don't?

I have tried with a bigger dialog, I felt it was creating too much distraction

That's fair. There's already a fair bit of whitespace, and that could perhaps be more evenly distributed, but good enough for now.

Maybe enabling the scroll would be a better tradeoff?

I hadn't realised it scrolls when you click it. I guess I didn't interact with it enough. That's good enough.

@oliviertassinari
Copy link
Member Author

Or why Google and Ant don't?

I don't know. Ok, let's change it. I have no idea which approach would be more useful, if you have a conviction it's better, let's go for it!

@mbrookes
Copy link
Member

Ok, let's change it.

Done. Figured since I asked for the change that I should do the work. Hope I got it right! 😅

@mbrookes
Copy link
Member

One thing I noticed working with the smaller set of icon imports is that of course the sort order is affected by the suffix. Not sure there's much we can do about it!

@oliviertassinari
Copy link
Member Author

Awesome, I was about to give it a spin, perfect timing ✨.

@oliviertassinari oliviertassinari merged commit b1ed501 into mui:master Aug 14, 2019
@oliviertassinari oliviertassinari deleted the icon-search branch August 14, 2019 23:36
aloiret pushed a commit to aloiret/material-ui that referenced this pull request Aug 15, 2019
* [docs] Make it easier to find material icons

* Josh review

* Matt feedback, add ellipsis

* Use radio for icon themes

* add josh suggestion
aloiret pushed a commit to aloiret/material-ui that referenced this pull request Aug 16, 2019
* [docs] Make it easier to find material icons

* Josh review

* Matt feedback, add ellipsis

* Use radio for icon themes

* add josh suggestion
@eps1lon
Copy link
Member

eps1lon commented Aug 20, 2019

This looks awesome 😍

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[docs] Implement a search + individual pages for the icons
5 participants