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

Allowing custom icons in the DocCards #9268

Closed
wants to merge 4 commits into from

Conversation

tim-vandenbosch
Copy link

@tim-vandenbosch tim-vandenbosch commented Aug 28, 2023

Pre-flight checklist

  • First time contributor
  • I have read the [Contributing Guidelines on pull requests] (https://github.com/facebook/docusaurus/blob/main/CONTRIBUTING.md#pull-requests).
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

When creating a separated blog I wanted to have custom icons of instead the 3 available.
This way a page can determine by just adding in the heading
sidebar_custom_props: customIcon: 🚀

Test Plan

No bonus points for me, I haven't been able to make the repo run for me.
I'll be making help requests in the discord server.

Test links

Deploy preview: https://deploy-preview-9268--docusaurus-2.netlify.app/

Related issues/PRs

I just started to code, should I make an issue for this?

@netlify
Copy link

netlify bot commented Aug 29, 2023

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 1ec6e8c
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/64ed9280e5d0360008b9ed4e
😎 Deploy Preview https://deploy-preview-9268--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions
Copy link

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 52 🟢 97 🟢 92 🟢 100 🟠 89 Report
/docs/installation 🟠 75 🟢 98 🟢 92 🟢 100 🟠 89 Report

@@ -85,7 +85,7 @@ function CardCategory({
return (
<CardLayout
href={href}
icon="🗃️"
icon={(item.customProps?.customIcon as ReactNode) || '🗃️'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

customProps is for props that are provided by the end user.

If we were to implement this feature, we wouldn't use customProps but rather make it a first-class feature by adding some kind of item.icon to our validation schemas (sidebar items + frontMatter)

@slorber
Copy link
Collaborator

slorber commented Aug 31, 2023

I understand the need for this, but unfortunately don't want to merge this for various reasons exposed here:

Overall what you propose is a good idea for your own site, that you can apply through swizzling. But it does not look like the best long-term option for an official public Docusaurus API.

Also we want to normalize how we use icons in Docusaurus in the future. It's likely that we'll migrate away from emojis in the future, so I'd rather not introduce any new API that could only work with emojis.


So, I'm going to close this PR for now, but we'll keep thinking about how to make your use case easier.

One idea in the future could be to have an DocCard/Icons/index.ts with a map of all the available icons, and passing sidebarItem.icon would pick an entry of this map. This would allow to support SVGs as well.

If you want to work on this, or any Docusaurus feature introducing a new public API, it's preferable to open a proposal before so that we can discuss the details.


No bonus points for me, I haven't been able to make the repo run for me.

Maybe our onboarding docs are not up to date, what did you try and what blocked you?

@slorber slorber closed this Aug 31, 2023
@slorber slorber added the closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests) label Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA closed: wontfix A fix will bring significant overhead, or is out of scope (for feature requests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants