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

feat: add plugin-icon #319

Merged
merged 11 commits into from
Dec 26, 2024
Merged

feat: add plugin-icon #319

merged 11 commits into from
Dec 26, 2024

Conversation

Mister-Hope
Copy link
Member

No description provided.

@Mister-Hope Mister-Hope changed the title feat: add plugin icon feat: add plugin-icon Dec 22, 2024
@Mister-Hope Mister-Hope marked this pull request as ready for review December 25, 2024 16:30
@Mister-Hope
Copy link
Member Author

Mister-Hope commented Dec 25, 2024

@pengzhanbo Please review this and see if you can accept to use this plugin in your theme. Our goal is to generate a usable icon plugin across themes.

I don't think adding full package support for iconify is needed.

  1. The full package is too large, and it's fast enough to access icons from iconify apis.
  2. Icon is definitely NOT important for SSR and SEO.

So I would suggest using the recommend way that iconify icon suggest, rather than supporting https://theme-plume.vuejs.press/guide/features/icon/#%E5%8A%A0%E8%BD%BD%E5%9B%BE%E6%A0%87

Note: We can support iconify-icon package as well, but it has little benefit as users are already allowed to use any CDN links or even self host the js file and specific iconify type manually.

@Mister-Hope
Copy link
Member Author

Pinging @recoluan for your theme-reco as well

@vuepress vuepress deleted a comment from netlify bot Dec 25, 2024
@vuepress vuepress deleted a comment from coveralls Dec 25, 2024
@recoluan
Copy link
Contributor

Pinging @recoluan for your theme-reco as well

Good job!

@pengzhanbo
Copy link
Member

@Mister-Hope First of all, this plugin is great, but I have reservations about fully loading icon resources via CDN. In my practical application scenarios, as well as feedback from some users, it is quite common to have completely isolated external network connections within corporate intranets, which makes it impossible to load icon resources correctly in such cases. On the other hand, when there are a large number of icons present in the visible area of the site, it triggers a significant number of icon requests. Due to the browser's limitations on concurrent requests, this can lead to frequent UI flickering.

@Mister-Hope Mister-Hope merged commit 09fec63 into main Dec 26, 2024
38 checks passed
@Mister-Hope Mister-Hope deleted the plugin-icon branch December 26, 2024 07:45
@Mister-Hope
Copy link
Member Author

Mister-Hope commented Dec 26, 2024

I decide to add this first. The next target is to support markdown grammar with it.

Then we can see if we can add built-in icons support for iconify.

For the grammar, I would prefer ::icon-name:: instead of :[icon-name]:, any reason that you choose :[xxx]: @pengzhanbo ?

@pengzhanbo
Copy link
Member

@Mister-Hope
::icon-name lacks a clear ending boundary, and icons often need to support settings for color, size, etc. Without a clear boundary, it is difficult to determine when to end.

:[xx]: references the emoji format :xxx:, allowing for more content to be written within []. This can be expanded to the syntax :[icon-name size/color]: to support settings for size and color.

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.

3 participants