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

Add Bootstrap Icons & Sort Sidebar Items #275

Merged
merged 9 commits into from Jan 30, 2020

Conversation

thatmattlove
Copy link
Contributor

@thatmattlove thatmattlove commented Dec 14, 2019

Hello!

Thank you for creating and maintaining this project, I find it extremely valuable. This PR should be mergeable as-is, but if not, I'd love to know of any other things that need to be done when adding an icon library. I have:

  • Added the submodule for Bootstrap Icons
  • Updated the manifest at packages/react-icons/src/icons/index.js and included the required glob filters for the reverse and fill variants
  • Updated the .gitignore for the built icons to include /bs/
  • Symlinked the forked react-icons library inside of package/preview/node_modules and verified that all the new icons appear properly
  • Bonus: fixed a preview/docs site issue that's bugged me for a while - sorted the sidebar navigation items :)

Thanks again!

PS: If the process I just followed is correct (or if not, and you let me know what's missing), I'd be happy to write up a brief contributing doc for future contributors. Just let me know!

@kamijin-fanta
Copy link
Member

thank you for your contribution. it's great work! If the conflict is cleared, merge.

@thatmattlove
Copy link
Contributor Author

Much appreciated! I don’t have write access to the repo, so I can’t merge. I believe the CI check failed due to not having the deploy api key in my environment variables.

@thatmattlove
Copy link
Contributor Author

Hi @kamijin-fanta - added a few more commits here as you can see. I noticed that you merged the RemixIcon PR (great icon set, glad to see it added!), and are moving the codebase to TypeScript, so my original commits needed some shifting around to fit in. I believe I've made all the necessary changes, as well as some extras on the Preview site. Looks like you've started moving to NextJS, so I moved my original CRA changes (sorting the sidebar) over to the Next config, and fixed an issue I found when trying to run the dev server locally (28b0196). Also added missing icon sets to the Readme and cleaned up some formatting. Hopefully this is helpful!

Thanks,
Matt

@kamijin-fanta
Copy link
Member

Thank you. Great contribution! CI will be modified in another PR.

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.

None yet

2 participants