indiana-university / rivet-icons

Icons for Indiana University's Rivet Design System
https://rivet.iu.edu/icons-stickers/
BSD 3-Clause "New" or "Revised" License
5 stars 6 forks source link

Version 3 Alpha #105

Closed basham closed 6 months ago

basham commented 1 year ago

Based on CodePen proof of concept.

basham commented 7 months ago

Please also compare the readme to the Rivet Stickers readme, as ideally both should be fairly well aligned. Perhaps one or both readmes may need to be adjusted so they better work together.

basham commented 7 months ago

I've made a number of changes since yesterday:

  1. All icon modules have moved from ./dist/icons to ./dist. This flattens the import paths.
  2. The icon bundle (./dist/rivet-icons.js) now imports rather than includes the Rivet Icon Element (rivet-icon-element.js). This ensures that you can't accidentally import two instances of the Rivet Icon Element. The bundle also will no longer re-export from the Rivet Icon Element.
  3. The readme has been rewritten to focus on particular usage scenarios (set up for development, set up for production, making it accessible).

Questions:

  1. The readme no longer mentions rivet-icons.json. I feel like the only real usage of this file is for servers rending the full page of icons, which would only be used for the Rivet docs (maybe?) and this repo's test environment. Should anything be done? Is index.json a better name?
  2. I added a section to the JavaScript API about the exported name and svg values from icon modules. This also seems like a bit of a stretch for use cases. Should this be left undocumented? Should those exports be removed and added back later on if new use cases come up?
  3. Should the icon bundle (rivet-icons.js) be renamed to something else to make it more obvious what it is or how it should be used? For example: index.js, rivet-icons.dev.js, development.js
basham commented 6 months ago

This repo is now aligned with the stickers work (https://github.com/indiana-university/rivet-stickers/pull/2). So, this is ready for review for a v3 release.

Once the Rivet docs are updated, we need to update the repo's external link to the new URL: https://rivet.iu.edu/icons-stickers/

levimcg commented 6 months ago

@basham @scottanthonymurray Yay! This is exciting. 😄

I would add the same comment I did on the rivet-stickers repo re: shipping a fall-back rivet-icons.min.js file in the /dist folder here.