search

levrik / mdi-react

Material Design Icons for React/Preact packaged as single components
Other
147 stars 23 forks source link

mdi-text-box icons are missing #75

Closed masbaehr closed 4 years ago

masbaehr commented 4 years ago

Hi, just updated to the shiny new version, but some things are not right now:

https://github.com/Templarian/MaterialDesign/issues/4901

For example all codepoints have been changed and some icons for example "TextBoxIcon" now produce the wrong icon according to

http://materialdesignicons.com/cdn/5.0.45/

levrik commented 4 years ago

I don't fully get what the issue is here. Does TextBoxIcon in mdi-react differ from what you can see in the link you've posted?

masbaehr commented 4 years ago

Hm i expected it to be like this: image

But it seems that the actual error/confusion might not be in mdi-react but in materialdesignicons. Sorry for the trouble caused. Just to understand: When they change something in the codepoint, for mdi-react this does not matter? Then the issue can actually be closed

masbaehr commented 4 years ago

Strange thing i noted: image

According to this there should actually be two textbox icons 'mdi-react/TextBoxIcon' and 'mdi-react/TextboxIcon'

levrik commented 4 years ago

The codepoints are used with the icon font. mdi-react exposes the SVG icons. The mdi-text-box icon should be converted to TextBoxIcon and mdi-textbox to TextboxIcon. Which doesn't really work for case insensitive file systems. But also from materialdesignicons' side this seems quite confusing. Maybe open an issue over at their repo. If they don't want to change this I'll think about a different naming conversion. I'll keep the issue open for now.

masbaehr commented 4 years ago

Maybe for providing a temporary fix to this you could just make an exception for this one. An ugly but working solution is still better than no solution :) As far as i have seen, this is the ONLY case with this name clash

For example: TextboxInputIcon

Probably at materialdesignicon they just think about css-classes when deciding for a name

Templarian commented 4 years ago

@levrik You can close this by the way. This was fixed upstream in 5.1+ months ago. form-textbox is the prefix. This was to prevent enum issues with TEXTBOX in other third party tooling. 👍

We also have added checks to prevent name collisions in the future.

levrik commented 4 years ago

@masbaehr Can you confirm this to be fixed in newer versions?

masbaehr commented 4 years ago

I think it is fine now thus im closing the issue. Although i migrated to mdi-material-ui in the meantime as it supports tree-shaking