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

Add checkboxes to select each font family on the directory website #948

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

slestang
Copy link

@slestang slestang commented Feb 13, 2019

This has become required because the number of families keeps growing and the number of icons inside each family too, making the page very long.

The design is a little rough, feel welcome to improve it.

May be at the beginning we should unselect all families so the page will be faster to load?

I think it depends if people usually use all families or just a few.

Copy link
Collaborator

@hampustagerud hampustagerud left a comment

Choose a reason for hiding this comment

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

Most of it looks good!

I doubt that a lot of people use more than a few families so I think having all families disabled at first is a good idea, it saves several seconds of loading the website 🙂 I think there should be a separate message that replaces Icon not found if no families are selected.

It bothers me a little that the Brands part of FontAwesome5 is shown as a separate font. Maybe each style should be a separate font but for now it's better to have them all unified, just filter out FontAwesome5Brands from the array of keys 🙂

Also, maybe there should be a Select all and Select none button to make it easy to enable/disable all fonts? 🤔

directory/src/App.js Outdated Show resolved Hide resolved
directory/src/App.js Outdated Show resolved Hide resolved
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