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

feat: add icon font files #10996

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from
Open

feat: add icon font files #10996

wants to merge 22 commits into from

Conversation

allieorth
Copy link

@allieorth allieorth commented Dec 5, 2024

Related Issue: #56

Summary

There has been multiple requests for a font using the Calcite UI icons. This is filling that ask.
There are 3 font files and corresponding json files to organize and standardize their codepoints.
Also contained in this folder is a codepoints.json for use when updating the font (which will happen every 2 months in line with the release schedule) to keep the codepoints increasing numerically and avoid scrambling codepoints when adding new icons.

Lastly is the svgs — these are included as they differ from the repo’s svgs because we have to remove icons with multiple paths i.e. opacity.

Take a look at what I have here and let me know if there are improvements to be made. (I am unsure if the SVGs should be in the repo or only on our server, as here they are essentially duplicates?)

I am also hoping to get the codepoints converted to Unicode and included in the searchable ref page to make the font characters searchable. I’m working with @macandcheese on that [here]

`calcite-ui-icons-16.ttf`, `calcite-ui-icons-24.ttf`, `calcite-ui-icons-32.ttf`
These are the 3 source files for the icon font, they contain all the single path artwork at each size. The individual sizes are to encourage users to use the font at the intended pt/px size.
codepoints.json for use when updating the font to keep the codepoints increasing numerically and avoid scrambling codepoints when adding new icons.
these svgs differ from the icon repo svgs since any icon with multiple paths have been removed (it breaks the font)
these svgs differ from the icon repo svgs since any icon with multiple paths have been removed (it breaks the font)
these svgs differ from the icon repo svgs since any icon with multiple paths have been removed (it breaks the font)
@github-actions github-actions bot added the enhancement Issues tied to a new feature or request. label Dec 5, 2024
@calcite-admin calcite-admin added the skip visual snapshots Pull requests that do not need visual regression testing. label Dec 5, 2024
Copy link
Contributor

@arowles arowles left a comment

Choose a reason for hiding this comment

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

Looks good to me! @benelan and @jcfranco, it will probably be best to get your 2 cents since there is a lot of files

Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

Thanks for putting this together! I have some questions before moving forward with the PR:

  1. Should any of these files be added to the NPM package?
  2. How were the font files created, is there a script somewhere? If so, we should:
    • add the script(s) to lib/
    • document the build process and/or add the required commands as an npm script

I'm hesitant to commit the mostly duplicate SVGs to the monorepo. I'm sure there's a way to programmatically exclude icons with multiple paths. If you can provide more information on the font's build process I should be able to assist with scripting a solution.

I also have a couple initial change requests:

  1. Can you change font/ to fonts/ so it's aligned with icons/
  2. Can you move the files in fonts/font files/ to fonts/ unless there's a good reason for the extra directory

removing svg files that are essentially duplicates
Will either host these on creative lab server, or work with Ben to script a solution
removed extra unnecessary directory
changed `font/` to `fonts/` so it's aligned with `icons/`
@macandcheese
Copy link
Contributor

From a doc standpoint - I think we should be a bit defensive here, with any public display. To date, from the component side, we’ve taught folks to use icons via the component and built-in properties, etc - so even if this isn’t “advertised” publicly, adding some guardrails would be nice.

I also think this strategy of three “sizes” of fonts might need a bit of documentation. As a user - I might expect to use a single icon font file that I can use at any font size. While this is at odds with the “three fixed sizes” approach to some degree, it would be nice to outline our expectations with these.

@calcite-admin calcite-admin requested a review from a team December 12, 2024 00:53
@calcite-admin calcite-admin added the calcite-ui-icons Issues specific to the @esri/calcite-ui-icons package. label Dec 12, 2024
@benelan benelan self-requested a review December 12, 2024 02:39
Copy link
Member

@benelan benelan left a comment

Choose a reason for hiding this comment

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

This LGTM after I automated the build process and reduced the files being committed to the repo.

When building or starting the dev server, the bin/build-fonts.sh script will execute, which:

  • moves the svg icons into intermediary directories by size, filtering out icons with opacity
  • builds the ttf font files
  • updates the fantasticonrc.json file with the codepoints for any new icons that are added.

The fonts will be bundled into the npm package, which means users will be able to access them from npm or from the esri-calcite-ui-icons-<VERSION>.tgz asset attached to the UI Icon's GitHub Releases.

@arowles - moving forward, make sure the fantasticonrc.json file is updated whenever new icons are added. This should happen automatically when running the dev server, but make sure to commit the changes to the file.

@allieorth - please do one more round of testing on your end, and if everything looks good feel free to merge. You won't need to do anything on your end before releases, it's all been automated! If you have questions or concerns, feel free to reach out here or on Teams. Thanks!

Copy link
Member

@jcfranco jcfranco left a comment

Choose a reason for hiding this comment

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

🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠
🔠⚛️🔠🔠🔠⚛️🔠🔠⚛️⚛️🔠🔠⚛️⚛️⚛️🔠🔠⚛️⚛️⚛️🔠⚛️⚛️⚛️⚛️🔠⚛️🔠
🔠⚛️⚛️🔠🔠⚛️🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠🔠🔠⚛️🔠🔠🔠🔠⚛️🔠
🔠⚛️🔠⚛️🔠⚛️🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠🔠🔠⚛️⚛️⚛️🔠🔠⚛️🔠
🔠⚛️🔠🔠⚛️⚛️🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠⚛️🔠🔠🔠🔠⚛️🔠🔠🔠🔠🔠🔠
🔠⚛️🔠🔠🔠⚛️🔠🔠⚛️⚛️🔠🔠⚛️⚛️⚛️🔠🔠⚛️⚛️⚛️🔠⚛️⚛️⚛️⚛️🔠⚛️🔠
🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠🔠

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
calcite-ui-icons Issues specific to the @esri/calcite-ui-icons package. enhancement Issues tied to a new feature or request. skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants