Skip to content

✨ NEW: Add material design icons roles #41

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.

Sign up for GitHub

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

Jump to bottom
Merged
chrisjsewell merged 13 commits into from
Dec 16, 2021
Merged

✨ NEW: Add material design icons roles #41

chrisjsewell merged 13 commits into from
Dec 16, 2021
+131,785 −8

Conversation

@2bndy5
Copy link
Contributor

@2bndy5 2bndy5 commented Nov 13, 2021 •
edited
Loading

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 •
edited
Loading

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

sphinx-design/tests/test_snippets.py

Line 102 in 629b868

file_regression.check(

@chrisjsewell chrisjsewell mentioned this pull request Nov 13, 2021
Closed
@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021 •
edited by codecov bot
Loading

Codecov Report

❌ Patch coverage is 83.63636% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.51%. Comparing base (63754db) to head (de347e2).
⚠️ Report is 75 commits behind head on main.

Files with missing lines Patch % Lines
sphinx_design/icons.py 83.63% 9 Missing ⚠️

❌ Your project status has failed because the head coverage (88.51%) is below the target coverage (90.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main      #41      +/-   ##
==========================================
- Coverage   88.82%   88.51%   -0.32%     
==========================================
  Files          10       10              
  Lines         859      914      +55     
==========================================
+ Hits          763      809      +46     
- Misses         96      105       +9
Flag Coverage Δ
pytests 88.51% <83.63%> (-0.32%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

2bndy5
2bndy5 commented Dec 16, 2021
View reviewed changes
2bndy5
2bndy5 commented Dec 16, 2021
View reviewed changes
chrisjsewell
chrisjsewell approved these changes Dec 16, 2021
View reviewed changes
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

chrisjsewell commented Dec 16, 2021

Sounds good ta 👍

@chrisjsewell
Copy link
Member

chrisjsewell commented Dec 16, 2021

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

chrisjsewell commented Apr 13, 2022

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

Reviewers

@chrisjsewell chrisjsewell chrisjsewell approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Is there a plan to add material design inline icons?

3 participants

@2bndy5 @codecov-commenter @chrisjsewell