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

Material Icons seems to be missing many common icons, e.g. 'arrow_back' #229

Closed
thomas5280 opened this issue Feb 19, 2023 · 7 comments
Closed
Labels
good first issue Good for newcomers invalid This doesn't seem right

Comments

@thomas5280
Copy link

Hello,

Thank you for this functionality.

I'm simply trying to extract a subset of Material Icons to a woff2 file. Things generally seem to be working fine, and some icons export properly.

I'm using 'npm run demo' with custom modified icon subset in demo-standalone.js modified directly in /dist folder after running build.

Many icons appear to be missing, here are examples:
[MiProvider] warn: "arrow_back" is ignored, reason: "arrow_back" does not exists in metadata available.
[MiProvider] warn: "rotate_90_degrees_ccw" is ignored, reason: "rotate_90_degrees_ccw" does not exists in metadata available.
[MiProvider] warn: "skip_previous" is ignored, reason: "skip_previous" does not exists in metadata available.
[MiProvider] warn: "skip_next" is ignored, reason: "skip_next" does not exists in metadata available.
[MiProvider] warn: "folder_zip" is ignored, reason: "folder_zip" does not exists in metadata available.
[MiProvider] warn: "file_download" is ignored, reason: "file_download" does not exists in metadata available.
[MiProvider] warn: "more_vert" is ignored, reason: "more_vert" does not exists in metadata available.

But, all of these show up correctly in @marella's demo page,
https://marella.me/material-icons/demo/

e.g., typing in 'arrow_back' into the search there clearly shows the icon.

I've gone through the code, and my understanding is that we link dynamically to the latest material-icons that was installed form NPM (and the SVGs, too).

The only thing I can think of is, the demo is using very old versions of the material icon libraries. Checking in node_modules, I see:

128352 Feb 18 18:15 material-icons.woff2

so these seem to be written fairly recently, but I'm wondering if they're old versions in case material-icons has not been updated. But it seems to be.

In any case, I must be doing something wrong. If not, then we probably need to make sure we're using the latest icon libraries.

Can you offer any suggestions for me?

Thank you

@thomas5280
Copy link
Author

For example, 'arrow_back' is clearly in the metadata in that release from 2022:
https://github.com/marella/material-icons/blob/7bc5cf35e0ac7cab1c9c9860e7719993806d0f92/index.d.ts

is there some bug in the metadata parsing in subset-iconfont?

@dzhuang
Copy link
Owner

dzhuang commented Feb 19, 2023

Thanks for reporting. According to this line, we were using the minified version of the css file to get the unicodes of selected icons.

However, in the min.css file in the release (the zip files), the arrow-back icon just doesn't exist. I think it should be the bug of the material-icons packaging, and I can also "improve" this by reading the unicode from css file instead of the minfied one, but I need sometime to do this. Thanks for reporting.

@thomas5280
Copy link
Author

Thank you for the investigation and for confirming.

This is really interesting. I think it's an issue with "arrow_back" vs "arrow-back"

In their latest release in the ZIP file,
'arrow-back' appears to be present in the material-icons.min.css, but not "arrow_back"

But only "arrow_back" is in the code points file, and "arrow-back" is not there in the codepoints.json.

According to Google, the "arrow_back" is correct.
See: https://fonts.google.com/icons?icon.query=arrow_back&icon.set=Material+Icons

So this almost certainly seems to be a bug in a mismatch with hyphens vs underscores

@thomas5280
Copy link
Author

Thanks for reporting. According to this line, we were using the minified version of the css file to get the unicodes of selected icons.

However, in the min.css file in the release (the zip files), the arrow-back icon just doesn't exist. I think it should be the bug of the material-icons packaging, and I can also "improve" this by reading the unicode from css file instead of the minfied one, but I need sometime to do this. Thanks for reporting.

Note, it's incorrect in the non-minified version too,
.mi-arrow-back::before {
content: "\e5c4";
}

I think the fix may be to read the codepoint from the codepoints.json file?

@thomas5280
Copy link
Author

Temporary workaround:

  1. Paste in codepoints.json file raw contents as a hard-coded map in material_icons.js:
var ICON_CODEPOINT_MAP = {
    "123": "eb8d",
    "360": "e577",
/// .... etc etc etc 
}
  1. This code augments the map when processing:
        for (const key in ICON_CODEPOINT_MAP) {
          if (ICON_CODEPOINT_MAP.hasOwnProperty(key)) {
            // console.log(`${key}: ${ICON_CODEPOINT_MAP[key]}`);
            ret[key] = { unicode: ICON_CODEPOINT_MAP[key] };
            // console.log(ret[key]);
          }
        }
        ```

@dzhuang
Copy link
Owner

dzhuang commented Feb 19, 2023

Thanks for the investigation.

I think the fix may be to read the codepoint from the codepoints.json file?

Great! I think that should be the solution. If I am not wrong, the codepoints.json was not included in the release when the package was cooked. Can you submit a patch or wait for quite a while as I am currently buried.

@dzhuang dzhuang added help wanted Extra attention is needed good first issue Good for newcomers labels Feb 19, 2023
@dzhuang
Copy link
Owner

dzhuang commented Jun 9, 2023

Hello @thomas5280

I apologize for the confusion caused by my previous replies regarding this issue. After further investigation, I have come to realize that this is not actually an issue, but rather a matter of correct usage.

To clarify, the appropriate usage for the icon name should be like arrow-back instead of arrow_back, i.e., please use hyphen instead of underscore as seperator.

Additionally, I understand that it might be helpful to have an alert or warning system in place when users input icon names with underscores. This could help avoid any potential confusion or errors. I will make a note of this suggestion and pass it along to the relevant team for consideration.

Once again, I apologize for any confusion caused, and thank you for bringing this matter to our attention. If you have any further questions or concerns, please don't hesitate to let me know.

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 invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants