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

Support Google Fonts Icons #1681

Merged
merged 18 commits into from
Oct 23, 2021
Merged

Conversation

riyadh-h
Copy link
Contributor

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Resolves #1505

Overview of changes:
Add support for Google's Material icons using the material-icons package.

Anything you'd like to highlight / discuss:
The package used is not an official package by Google, since Google does not mention how the icons can be added as an npm package in the developer guide.

  • Note: Google used to explain how the icons can be added using the material-design-icons npm package, but that package appears to be outdated in terms of including all icons and styles shown in the official icons' website. The material-icons npm package, however, is up-to-date with Google's latest set of icons and styles.

Testing instructions:
n/a

Proposed commit message: (wrap lines at 72 characters)

Support Google Fonts Icons

MarkBind does not support the inclusion of Google's Material icons
using the two-colon syntax (e.g., `:icon-name:`).

Users who want to use icons from that library would need to include
`link` HTML elements per MarkBind project and write lengthy `span` HTML
elements per icon, which can be inconvenient if done frequently.

Let's extend MarkBind's existing two-colon icon syntax to support
Google's Material icons.

Using the two-colon syntax would make the usage of Google's Material
icons much easier, as it does not require the users to manually use
HTML to include said icons.

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes
    • Some changes were automatically made to a few package-lock.json files while adding the material-icons npm package.

@riyadh-h riyadh-h marked this pull request as ready for review October 10, 2021 12:23
Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Thanks @riyadh-h for working on this 😃. Tried it out and it looks good 👍

I might be missing something but it seems that the material-icons package is only used within Markbind's core package. I believe we can remove this dependency from the package.json of cli, core-web and vue-components.

@@ -37,6 +37,7 @@
"fs-extra": "^9.0.1",
"live-server": "1.2.1",
"lodash": "^4.17.15",
"material-icons": "^1.9.1",
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove the material-icons dependency from here since it does not seem to be used in this package? Same for core-web and vue-components.

@riyadh-h
Copy link
Contributor Author

Thank you for the review, @jonahtanjz, and I apologize for the late reply 😓

I might be missing something but it seems that the material-icons package is only used within Markbind's core package.

You're right, they are only used within the core package. I believe the extra dependency entries were automatically added when I installed the material-icons package using Lerna.

I'll try to remove them soon 👍

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Thanks @riyadh-h! :)

LGTM 👍

@jonahtanjz jonahtanjz added this to the 3.0.7 milestone Oct 23, 2021
@jonahtanjz jonahtanjz merged commit 3b12360 into MarkBind:master Oct 23, 2021
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.

Support Google Fonts Icons
2 participants