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: Ensure icons in the private use area (fixes #365) #772

Closed
wants to merge 1 commit into from

Conversation

earboxer
Copy link
Contributor

@earboxer earboxer commented Jan 26, 2022

Description

This is following @delphinus "Plan 2" in #365

Requirements / Checklist

What does this Pull Request (PR) do?

How should this be manually tested?

Build the font and notice that:

  • nf-mdi-access-point is still F501 
  • E802 is a pushpin (which was previously F902)

Any background context you can provide?

It should be noticed that the material icons we are using are an older version of https://github.com/Templarian/MaterialDesign (it now has 6675 icons).

Google also has a newer version at https://github.com/google/material-design-icons (They have 2142 code points in their Regular font (no variants), which includes some codepoints with semantic meaning but identical appearance (our nf-mdi-sim_alert looks the same as their sim_card_alert and sd_card_alert))

both of these have added several new icons, but they've also removed icons such as nf-mdi-pinterest and nf-mdi-pinterest_box (which notably are redundant with the fontawesome icons nf-fa-pinterest_p and nf-fa-pinterest_square).

While the merge request is moving the code points, you could view this merge request as doing two things:

  1. Removing bugged icons
  2. Adding new icons (and assigning them to new codepoints).

What are the relevant tickets (if any)?

Fixes #365

@earboxer earboxer changed the title Draft: Material Icons: Keep icons in the private use area Material Icons: Keep icons in the private use area (fixes #365) Jan 26, 2022
@earboxer earboxer marked this pull request as ready for review January 26, 2022 22:30
@earboxer earboxer changed the title Material Icons: Keep icons in the private use area (fixes #365) Material Icons: Ensure icons in the private use area (fixes #365) Jan 26, 2022
@Finii
Copy link
Collaborator

Finii commented Jan 27, 2022

This just is the technical aspect, all the needed documentation files are missing in the PR.
If this would be pulled someone else would need to fix the documentation afterwards?
It should all be in ONE commit.

Edit: Docu is not reachable via PR ...

@Finii
Copy link
Collaborator

Finii commented Jan 27, 2022

This MR does not implement Plan Plus as per #365 (comment).
Which I personally find unfortunate, but nothing has been discussed yet.

But I like this MR, as it finally tries to get some momentum going in this issue :-)

One question for @earboxer: You say there are new/updated Material icons available. Did you by chance check if they move codepoints? Font Logos does it :-( (lukas-w/font-logos#72).

@earboxer
Copy link
Contributor Author

@Finii Yeah, they (Templarian) moved the codepoints to starting at 0f0001.

It looks like the layout is roughly the same, but many breaking changes (replaced icons). e.g. 0f002e is now a pair of rollerblades instead of nf-mdi-amazon_clouddrive (which was f02e in the older source).

But then when we get to the bottom of the code page, nf-mdi-home_heart is at 0F0827, where it was previously at F826.

Scrolling back through the icons, it seems the 'off-by-one' discrepancy between the old and new sheets starts at F675 (which is now starting at 0F0676), nf-mdi-credit_card_plus.

(This is comparing our installed file against v6.5.95, downloaded from their website)

@earboxer
Copy link
Contributor Author

@Finii What all needs to be updated? I see bin/scripts/lib/i_material.sh Is there anything else?

@Finii
Copy link
Collaborator

Finii commented Jan 27, 2022

What all needs to be updated?

I thought about the documentation, but as I try to link here, that is in another repo / wiki...
I'll strike the comment.

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.

[Suggestion] Fix invalid code points for some glyphs
2 participants