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

Icons Reference #225

Closed
ygorma opened this issue Nov 9, 2021 · 23 comments
Closed

Icons Reference #225

ygorma opened this issue Nov 9, 2021 · 23 comments
Labels
good first issue Good for newcomers

Comments

@ygorma
Copy link

ygorma commented Nov 9, 2021

As a recomendation, I think it would be nice to include in the Icons page, a link redirecting to the google fonts site in order to make the search for Icons easier via tags, something that it's not supported via Control + F in the icons page. And you may have realized that not all the Icons are present in that page.

Here is the link: https://fonts.google.com/icons?selected=Material+Icons

@LoganTann
Copy link

Good suggestion, we'll see what other contributors thinks

@DanielRuf DanielRuf added the good first issue Good for newcomers label Nov 16, 2021
@DanielRuf
Copy link

PRs are very welcome.

I think we should replace this long list with a simple link, as you correctly mention the outdated list in our docs.

@LoganTann
Copy link

I think we should replace this long list with a simple link, as you correctly mention the outdated list in our docs.

I'm often in the docs while developing with materialize. Isn't providing both is better than a page with a single link ?

@DanielRuf
Copy link

I'm often in the docs while developing with materialize. Isn't providing both is better than a page with a single link ?

Generally that would mean we have to actively maintain the list.

@MadhaviMandalia
Copy link
Member

I want to contribute, please let me know what should be done here? Should I make changes to the list or just provide a link to the icon page?

@DanielRuf
Copy link

Hi @MadhaviMandalia,

Thank you very much for asking.

just provide a link to the icon page?

That should be the right solution.

PRs are very welcome.

@MadhaviMandalia
Copy link
Member

What is the approach you here? Where should I include the link on the icon page?

@DanielRuf
Copy link

DanielRuf commented Dec 21, 2021

What is the approach you here? Where should I include the link on the icon page?

You can safely remove the current list and add a text with the relevant link at the same place.

It doesn't have to be perfect. You will get some feedback like change suggestions during the PR review phase. When everything is fine we will merge and deploy the changes.

@MadhaviMandalia
Copy link
Member

I have removed the icon list but there is already 'Google Icon' link on right hand side marked as Search Google Icons so please let me know what changes should I perform.

image

@DanielRuf
Copy link

I have removed the icon list but there is already 'Google Icon' link on right hand side marked as Search Google Icons so please let me know what changes should I perform.

This looks good so far.

I would change this:

We have included 932 Material Design Icons courtesy of Google. You can download them directly from the Material Design specs.

To something like that:

We use Google Material Icons by Google. They provide a searchable list here (which we do not include in the documentation here), which also shows you the relevant icon names for the CSS classes. You can download the icons directly from the Material Design specs.

Feel free to improve or use a different wording.

@ChildishGiant
Copy link
Member

Might be worth pointing to MDIDX over the official repo as it's far more supported.

@MadhaviMandalia
Copy link
Member

Ok I will include this one.
Thanks.

@ygorma
Copy link
Author

ygorma commented Dec 21, 2021

I have removed the icon list but there is already 'Google Icon' link on right hand side marked as Search Google Icons so please let me know what changes should I perform.

image

I think it's still too discret..

The list should be removed.. It does not include all the icons and may induce the user to think that all the available icons are there and hard to seek without tags, or related words.

@ygorma
Copy link
Author

ygorma commented Dec 21, 2021

I have removed the icon list but there is already 'Google Icon' link on right hand side marked as Search Google Icons so please let me know what changes should I perform.

This looks good so far.

I would change this:

We have included 932 Material Design Icons courtesy of Google. You can download them directly from the Material Design specs.

To something like that:

We use Google Material Icons by Google. They provide a searchable list here (which we do not include in the documentation here), which also shows you the relevant icon names for the CSS classes. You can download the icons directly from the Material Design specs.

Feel free to improve or use a different wording.

You don't need to download, they are already included within materialize.

@DanielRuf
Copy link

You don't need to download, they are already included within materialize.

It's a reference if you want to download them manually. Materialize does not include these icons. We just reference the Google Fonts there:

https://materializecss.github.io/materialize/getting-started.html

Bildschirmfoto 2021-12-21 um 18 09 39

The list should be removed..

That's what she currently does.

@MadhaviMandalia
Copy link
Member

So, should I include change the reference link or everything is fine?

@DanielRuf
Copy link

So, should I include change the reference link or everything is fine?

See my previous comments with the suggested changes.

@MadhaviMandalia
Copy link
Member

Hello @DanielRuf,
The previous changes were already committed and a PR is also raised for it. I was asking about the below comment that does it require any changes? If yes please let me know.

Thanks.

You don't need to download, they are already included within materialize.

It's a reference if you want to download them manually. Materialize does not include these icons. We just reference the Google Fonts there:

https://materializecss.github.io/materialize/getting-started.html

Bildschirmfoto 2021-12-21 um 18 09 39

The list should be removed..

That's what she currently does.

@DanielRuf
Copy link

DanielRuf commented Dec 31, 2021

I can not yet see any relevant PR at https://github.com/materializecss/materialize/pulls

What I remember and what your PR should contain:

  • remove the current list of icons
  • add link to the external list
  • improve / rephrase the text a bit

No extra changes needed regarding the quoted comment (as this was a clarification that materializecss does not include / ship the icon fonts).

Does this answer your question @MadhaviMandalia?

@MadhaviMandalia
Copy link
Member

MadhaviMandalia commented Dec 31, 2021

I have opened a PR in this repo please that check it out and let me know is there any change required
Here you go: #235

Earlier by mistake I've opened a PR in Dogfalo/materialize 😃

Regards.

@DanielRuf
Copy link

I have opened a PR in this repo please that check it out and let me know is there any change required
Here you go: #235

Awesome, will take a look later today.

DanielRuf pushed a commit that referenced this issue Jan 4, 2022
MadhaviMandalia added a commit to MadhaviMandalia/materialize that referenced this issue Jan 7, 2022
DanielRuf added a commit that referenced this issue Jan 7, 2022
* fix: add link to searchable list #225

* Update pug/page-contents/icons_content.html

Co-authored-by: Daniel Ruf <827205+DanielRuf@users.noreply.github.com>
@wuda-io
Copy link
Member

wuda-io commented Jan 11, 2022

Looks good, thanks for contributing!
I think its done, feel free to reopen if needed.

@wuda-io wuda-io closed this as completed Jan 11, 2022
@MadhaviMandalia
Copy link
Member

Hello @wuda-io & @DanielRuf ,
Thank you both 😃

DyegoCosta pushed a commit to DyegoCosta/materialize that referenced this issue Jun 27, 2022
DyegoCosta pushed a commit to DyegoCosta/materialize that referenced this issue Jun 27, 2022
* fix: add link to searchable list materializecss#225

* Update pug/page-contents/icons_content.html

Co-authored-by: Daniel Ruf <827205+DanielRuf@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

6 participants