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

Replace Google Material Icons with Material Symbols #2432

Merged
merged 26 commits into from
Jan 9, 2025

Conversation

96LawDawg
Copy link
Collaborator

@96LawDawg 96LawDawg commented Jan 4, 2025

This impacts games and the UI.

Needs a file updater and some visual testing of the UI because that stuff is everywhere. I think I got everything. But this was a bigger undertaking than expected because the default for the old Icons was "filled" and that had to be manually applied in some locations in the UI.

Material Icons have been deprecated and are no longer being updated. There are about 2100 Icons. Symbols are the new Google standard and are being updated regularly. There are about 3900 Symbols.

To Do:

  • File updater needs to change metadata
  • Broken add symbol button in metadata pages (problem on production, new PR for this)
  • Tags
  • Check as much of UI as possible for anything missed
  • Adapt overlay filter to show filled and not filled versions of Symbols

PR-SERVER-BOT: You can play around with it here: https://test.virtualtabletop.io/PR-2432/pr-test (or any other room on that server)

This impacts games and the UI.

Needs a file updater and some visual testing of the UI because that stuff is everywhere.  I think I got everything.  But this was a bigger undertaking than expected because the default for the old Icons was "filled" and that had to be manually applied in some location in the UI.
@96LawDawg
Copy link
Collaborator Author

There are subtle changes in how some icons/symbols display. Not really a big deal to me, but if this is a deal breaker, we could leave the icon font. Here is an example from the Editor toolbar on the right side of the screen. Original on top, new on bottom (look at history arrow and the bug).

Editor Toolbar Icons
Editor Toolbar Symbols

@96LawDawg
Copy link
Collaborator Author

There is one hopefully unimportant difference that I can't figure out. A single symbol gets replaced by a single symbol no problem. But if they are stringed together (like hieroglyphics) then the spacing is off. For example look at these 4 groupings in different font sizes. In each pair, the new Symbols are on the top. The old Icons are on the bottom.

Font Comparison Default Spacing

This is fixable, but if you fix it for a string of Symbols, then it breaks all individual symbols.

Font Comparison Adjusted Spacing

I looked at everything in the library and I did not see any instances of symbols strung together. So this is something I'm willing to live with as a possible breaking change.

Also, interestingly note the last symbol in each pair. It is called "new_releases." Used in Flatten the Curve. Changing from a check mark to an exclamation point is a little significant. But in that game it won't matter. Again, just an example of the risk of slightly different visual elements if we remove Icons.

@ArnoldSmith86
Copy link
Owner

Looks promising. Can you check if there is an equivalent file to the one read in https://github.com/ArnoldSmith86/virtualtabletop-misc/blob/main/helper-scripts%2Fgenerate-icon-list.sh#L8 ?

@scruffynerf
Copy link

Looks promising. Can you check if there is an equivalent file to the one read in https://github.com/ArnoldSmith86/virtualtabletop-misc/blob/main/helper-scripts%2Fgenerate-icon-list.sh#L8 ?

I looked hard, and there is no equiv, because the 'tags' piece seems like they no longer want to support it.
That said, I wonder if they are keeping the data updated, or if the newer icons just lack tags?

@scruffynerf
Copy link

I think you've shown that the new stuff is not a direct replacement of the old. So I'd say there is no harm in adding the new, but you'll want to keep the old.

@96LawDawg
Copy link
Collaborator Author

Looks promising. Can you check if there is an equivalent file to the one read in https://github.com/ArnoldSmith86/virtualtabletop-misc/blob/main/helper-scripts%2Fgenerate-icon-list.sh#L8 ?

It does exist. In a slightly different format here. This was on my internal to do list. This is current as of last month: https://github.com/google/material-design-icons/blob/master/update/current_versions.json

@96LawDawg
Copy link
Collaborator Author

Well, I guess that isn't exactly the same. It isn't tags. It is just a major category group.

@scruffynerf
Copy link

actually, I did some 'deep sleuthing' (I've been doing this stuff for way too long), dug into the guts of figma's google provided search (which has no repo), and discovered this undocumented url:
"https://fonts.google.com/metadata/icons?key=material_symbols&incomplete=true"
which is a larger file
I'd suggest you grab that

@96LawDawg
Copy link
Collaborator Author

Thanks @scruffynerf, that was really helpful. 50K lines of Material Symbols tags added. Some manual curating was required because there were a few that glitched. But this is so awesome now.

@96LawDawg
Copy link
Collaborator Author

@ArnoldSmith86, I think this is ready. But I can't remember about the file updater. Does it only work once everything is on the server. Because obviously the games are not updated yet and I can't figure out if it will actually work on not from testing on my local machine. Take a look and point me in the right direction if I'm off.

Copy link
Owner

@ArnoldSmith86 ArnoldSmith86 left a comment

Choose a reason for hiding this comment

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

Wouldn't it work to only replace the font file name and keep the font name "Material Icons" and keep all the class names?

Would be a bit confusing for people who actually know about the stuff. And I guess if this already works, it's fine. But the font names could be in use in any css property..

Maybe we just define both?

And maybe use a sledge hammer instead of a scalpel in the file updater and simply do a replace on the JSON string?

For the icons that changed: did you find alternatives that would be a better fit? We could replace those in the file updater as well.


function v17MaterialSymbols(properties) {
if (properties.type === 'classes') {
Copy link
Owner

Choose a reason for hiding this comment

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

properties is basically the widget JSON. So properties.classes and properties.id etc.

@ArnoldSmith86
Copy link
Owner

image
The rightmost three icons don't work properly, it seems.

And it looks like some categories are weird (probably some sort of duplicates)?
image

@scruffynerf
Copy link

And it looks like some categories are weird (probably some sort of duplicates)

Making all lower case would be best. (Brand sticks out)

@96LawDawg
Copy link
Collaborator Author

Making all lower case would be best. (Brand sticks out)

Good idea. But going with title case. A few of them in there work better as caps, so going to make it consistent.

@ArnoldSmith86
Copy link
Owner

For category capitalization etc. I would make them similar to the emoji categories because those are semi official I think..

@96LawDawg
Copy link
Collaborator Author

Wouldn't it work to only replace the font file name and keep the font name "Material Icons" and keep all the class names?

Not going to do that. Would make things easier now, but potentially much more confusing later.

But the font names could be in use in any css property..
And maybe use a sledge hammer instead of a scalpel in the file updater and simply do a replace on the JSON string?

I thought of that, but also thought you would prefer a scalpel. Technically, it could also be in routines. If could change something on a widget from normal text to a symbol and back by changing the class on a routine. The hammer should catch that.

For the icons that changed: did you find alternatives that would be a better fit? We could replace those in the file updater as well.

I don't know. I'm not really worried about it. Can't be that many and that big of a deal. Plus I don't know how to find them without comparing them all visually. I only noticed the one.

@scruffynerf
Copy link

There are still a few old icons (using custom made VTT symbols) leftover from the original JSON Editor. Now would be a good time to swap those out so our entire UI uses Google Material Symbols. Image below shows what we currently have on top and what I propose we use on bottom. From left to right

  • Copy state from another room
  • Macro
  • Show this widget below
  • Reverse order of F-key shortcuts
  • Duplicate widget
  • Open parent

Thoughts? The only one I really struggled with was open parent. We already have "tree" as button on the right-side menu bar and nothing else really fit. Of course, we could just leave them. VTT symbols aren't going anywhere.

this is solely about the 3 changed icons.

The robot arm doesn't scream 'macro' to me, ('automation' so I get the idea though).
the replacement looks like a Command Line, which also seems 'unmacro' like.
A 'keyboard' related icon would convey that it's a shortcut... didn't see one... but that sort of idea.

the Duplicate is MUCH better with the new icon. The stars didn't say "Duplicate to me" the 'copy' style icon does.

The open parent one is slightly less clear, but I wouldn't have know it was Open Parent from the arrow/line/circle combo either.
If you want to convey parent, use a parent... (an apparent parent, to be clear... a mostly dead parent with beautiful plumage is a bit much though). How about the icons for Supervisor Account (shows 2 figures, one larger behind it, which is what you are really saying...), or Escalator Warning, which shows a Parent holding a child's hand, and clearly you'd expect this to lead to the Parent.

@ArnoldSmith86
Copy link
Owner

I'm pretty sure all of the old ones are from Material Icons. Are they all not in Material Symbols? I don't mind changing the icons; I'm just wondering why so many are missing..

Also: the main toolbar uses VTT symbols as well. So unless we change those as well (and our logo won't be in Material Symbols), the font will be loaded anyway. So the icons you posted won't cause a font download either way.

It would at least make the editor UI icons more consistent but I don't feel like the old icons are standing out.

About the replacements: the macro one is fine but neither the old nor the new one is perfect.. The clone one is probably better. The parent ones are both pretty bad and unfortunately the tree icon is the wrong way around. I'm not sure how I feel about using people to show "parent" either. All those options are probably very similar: once you know, you can remember them but none of them convey the proper meaning at first glance.

I feel like it's better to not change them because at least current users might have gotten used to them. But I don't mind changing them at all.

That toolbar above the JSON editor buttons is probably the most out of place thing in the editor right now. It's kinda likely that it'll go away entirely (though the buttons might move somewhere else then). And in the long term the "proper" editor UI should be so good that you don't want to use the JSON editor at all. But all of that might be far into the future...

@ArnoldSmith86
Copy link
Owner

Both variants (fill and nofill) are now available in the symbol picker:

image

Apparently we now have 13288 symbols to pick from.

@ArnoldSmith86
Copy link
Owner

Can MaterialIconsOutlined-Regular.otf be removed now or is there a reason that it's still there?

@96LawDawg
Copy link
Collaborator Author

I feel like it's better to not change them because at least current users might have gotten used to them. But I don't mind changing them at all.

Then leave it as is. There are more reasons to leave it alone than there are to change it to satisfy some sort of sense of perfection to have them all be from the same set. (It was never about eliminating loading the 106kb VTT Symbols font). Also we'll never agree on the best parent symbol.

@96LawDawg
Copy link
Collaborator Author

Both variants (fill and nofill) are now available in the symbol picker:

I don't love this as much as I thought I would. I am glad to see both options, but the duplicates are distracting. Is there a way to have a checkbox or something that would filter for fill and no fill?

@scruffynerf
Copy link

scruffynerf commented Jan 8, 2025

Both variants (fill and nofill) are now available in the symbol picker:

I don't love this as much as I thought I would. I am glad to see both options, but the duplicates are distracting. Is there a way to have a checkbox or something that would filter for fill and no fill?

I wouldn't, as the difference in some is stark, and not in others. (and some not at all) And looking for an icon, having to flip the checkbox repeatedly seems awkward. If there are similar entries but 'inverted' (which is what fill often but not always does), I'd rather see them as separate choices (like now), than have to look harder.

@ArnoldSmith86
Copy link
Owner

I think I would prefer the current version. We can add a checkbox that hides the non-filled version. And if it is not checked, you get the current view.

@ArnoldSmith86
Copy link
Owner

There is one hopefully unimportant difference that I can't figure out. A single symbol gets replaced by a single symbol no problem. But if they are stringed together (like hieroglyphics) then the spacing is off.

Is this still an issue? Can you post an example room/game?

@96LawDawg
Copy link
Collaborator Author

There is one hopefully unimportant difference that I can't figure out. A single symbol gets replaced by a single symbol no problem. But if they are stringed together (like hieroglyphics) then the spacing is off.

Is this still an issue? Can you post an example room/game?

Yes. Can't do a side-by-side comparison in the same room now of course. But here is the same widget with a random collection of glyphs at 50px. The only difference is the first is on production and uses Icons. The second is in this PR and uses Symbols.

Icons
Symbols

@96LawDawg
Copy link
Collaborator Author

I think I would prefer the current version. We can add a checkbox that hides the non-filled version. And if it is not checked, you get the current view.

What I would like to see is a more feature-filled filter. When I type "home" into the search bar, I lose all the visual indication of which type of glyph this is. I almost never want emojis. Sometimes, I only want the game-icons.net ones. It would be great to have a filter near the search bar. Defaults to all, but uncheck and you remove all the emojis and material ones. But that can be another PR.

With or without the checkbox, I think this PR is good (unless somebody as issue with the spacing of a chain of widgets).

@96LawDawg
Copy link
Collaborator Author

You can approximate the spacing in the Symbols glyphs with this css "letter-spacing": "-0.37em". But as I said before, when I did that, it messed up the placement of single glyphs on some games. Like Unspeakables. Maybe I should test that again.

@96LawDawg
Copy link
Collaborator Author

So Unspeakables is not messed up if I add that change directly on the widget in the game. I'm at work now and can't test it locally. But my guess is if we add that css to the fonts.css where we have the default settings for symbols and symbols-nofill, it should fix the problem.

@ArnoldSmith86
Copy link
Owner

ArnoldSmith86 commented Jan 8, 2025

Hmm, the issue is that a space character in Material Symbols is as wide as an icon but in Material Icons it's way narrower. Fixing it with letter-spacing works but then it breaks when there's no space between two icons.

The only solution I can think of is to wrap the spaces with their own tag <span> </span> and then style that differently. That would be quite the pain though. Or to use letter-spacing and force either one space or no space between each icon.

The other solution is to not care. It is kinda likely though that somebody somewhere for some reason wanted to put two icons on a button.

If removing spaces is non-destructive, I would probably do that in the file updater. I'm gonna check if I can find an example where removing spaces would change the icons. For example add task to addtask. If addtask is its own icon, it would change. It is add_task though so that example would be fine.

Found an example: air play_arrow ▶️ airplay_arrow
image

@ArnoldSmith86
Copy link
Owner

I found word-spacing: -0.68em:

image

@96LawDawg
Copy link
Collaborator Author

I found word-spacing: -0.68em:

Not working for me on this example:

Icons
Symbols

@96LawDawg
Copy link
Collaborator Author

It is closer. And word-spacing works better than letter-spacing because if you don't have a space between the letters (something I didn't try), then you end up with the glyphs smooshed on top of each other with letter-spacing.

I think anything we try will be a bit of a hack and imperfect. We should figure out what word-spacing is best and apply it as the default. Dev's can then css stuff however they want.

It is really unlikely there are more than 2 or 3 symbols together. I didn't find a single instance in the PL where there was more than one, but there probably is and I missed it.

@96LawDawg
Copy link
Collaborator Author

I found that word-spacing: -0.75em worked perfectly on my example and your example and when our two examples were combined. I also did a couple more examples. I think the magic number is 0.75. I'm going to commit that now and do some more testing.

@96LawDawg
Copy link
Collaborator Author

96LawDawg commented Jan 8, 2025

Seems to work at -0.75. Video shows switching back and forth between PR with Symbols and production with Icons.

Edit: I also tried it on Firefox and got the same result. That had me worried for a minute when I was trying to figure out why .68 worked for you.

Symbol.Spacing.mp4

@96LawDawg
Copy link
Collaborator Author

I've looked again at the code, the UI, and a big sampling of PL games (with old icons). I think this is ready.

@ArnoldSmith86
Copy link
Owner

image

The outlier is Firefox on production for me. So it is better than before. If you don't have that behavior on Windows, even better. 🤷‍♂️

Copy link
Owner

@ArnoldSmith86 ArnoldSmith86 left a comment

Choose a reason for hiding this comment

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

We'll test the rest in production... 😁

@96LawDawg 96LawDawg merged commit 937b855 into main Jan 9, 2025
6 checks passed
@96LawDawg 96LawDawg deleted the feature-addGoogleMaterialSymbols branch January 9, 2025 22:05
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.

3 participants