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

✨ NEW: Add material design icons roles #41

Merged
merged 13 commits into from Dec 16, 2021

Conversation

2bndy5
Copy link
Contributor

@2bndy5 2bndy5 commented Nov 13, 2021

resolves #36

This PR add 5 new roles for using Google's Material Design Icons as inline text. Each role represents a different material design "flavor":

  • material-regular
  • material-outlined
  • material-sharp
  • material-round
  • material-twotone

Amongst all these flavors, there is a total of 10662 new icons provided. Instead of displaying them all, I defer users to browse Goggle's Material Design Icons showcase.

Includes updates to

  • docs
  • CSS (minified and SCSS)
  • tests (pre/post XML files)
    • For some reason the pytests about myst icons-material-design.xml fails. I suspect it has something to do with the bullets in the bullet list in the docs/snippets/myst/icons-material-design.txt or just a difference in LF vs CRLF (I develop in Windows - but also ran pytest in WSL ubuntu) πŸ˜•

Since the feature is working with the JSON files I programmatically generated from the google/material-design-icons repo, I set up a small repo to run a CI job that updates JSON files when needed.

@welcome
Copy link

welcome bot commented Nov 13, 2021

Thanks for submitting your first pull request! You are awesome! πŸ€—

If you haven't done so already, check out EBP's Code of Conduct and our Contributing Guide, as this will greatly help the review process.

Welcome to the EBP community! πŸŽ‰

@2bndy5
Copy link
Contributor Author

2bndy5 commented Nov 13, 2021

I still have no idea what's wrong with the pytest for Myst icons-material-design.xml.

I've tried using * as the bullets in the list, but that didn't help. I've tried manually editing both (pre/post) xml to not include the bullet="-" in the bullet_list tag, but then it complains that it is missing. πŸ€”

I'm not sure what is going on with the line that is triggering the AssertionError

file_regression.check(

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021

Codecov Report

Merging #41 (b11618d) into main (63754db) will decrease coverage by 0.53%.
The diff coverage is 80.00%.

❗ Current head b11618d differs from pull request most recent head f25a905. Consider uploading reports for the commit f25a905 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   88.82%   88.29%   -0.54%     
==========================================
  Files          10       10              
  Lines         859      914      +55     
==========================================
+ Hits          763      807      +44     
- Misses         96      107      +11     
Flag Coverage Ξ”
pytests 88.29% <80.00%> (-0.54%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Ξ”
sphinx_design/icons.py 78.52% <80.00%> (+0.74%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data
Powered by Codecov. Last update 63754db...f25a905. Read the comment docs.

docs/badges_buttons.md Outdated Show resolved Hide resolved
Copy link
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

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

Nice one cheers!

@chrisjsewell chrisjsewell changed the title [ENH, DOC, TST] support material design icons ✨ NEW: Add material design icons roles Dec 16, 2021
@chrisjsewell chrisjsewell merged commit 4a1a875 into executablebooks:main Dec 16, 2021
@welcome
Copy link

welcome bot commented Dec 16, 2021

Congrats on your first merged pull request in this project! πŸŽ‰
congrats

Thank you for contributing, we are very proud of you! ❀️

@2bndy5
Copy link
Contributor Author

2bndy5 commented Dec 16, 2021

I may submit updates (via PR) to the compiled jsons when I see significant changes to the material icons repo. These updates may come without a linked issue though.

@chrisjsewell
Copy link
Member

Sounds good ta πŸ‘

@chrisjsewell
Copy link
Member

Just going through some of the other issues, then should have a release out soon

@2bndy5
Copy link
Contributor Author

2bndy5 commented Apr 13, 2022

Any idea about ETA for next release?

I'm sorry this sounds impatient (there's really no need to rush), but I keep teasing this feature over on the sphinx-immaterial theme repo, so I'm very much looking forward to the next release. We're starting to consider an optional extra CSS that exports user-specified SVG's data (based on node.js pkgs - not google's src repo like this feature is) as CSS vars...

@chrisjsewell
Copy link
Member

No that's fair, sorry I got side-tracked with other things, but will try to get this done within the week

@2bndy5 2bndy5 deleted the material-icons branch April 21, 2022 10:52
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.

Is there a plan to add material design inline icons?
3 participants