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

fix(theme): Hide code block buttons before React hydration #10866

Merged

Conversation

kennethormandy
Copy link
Contributor

@kennethormandy kennethormandy commented Jan 24, 2025

Right now, code block “wrap” and “copy” buttons will show up in RSS feeds, and (as far as I’m aware) can’t be omitted using any feed config or existing settings.

Depending on the feed reader or the service consuming the feed, these empty <button> elements will still be treated as part of the content, alongside the code blocks.

Since these buttons don’t have any purpose outside of a browser-based context anyway, this PR removes those buttons unless they are being displayed in the browser.

Thanks!

Pre-flight checklist

  • I have read the Contributing Guidelines on 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

I’m sending my RSS feed to an email newsletter service, which imports the content.

Since the buttons are currently part of the feed content, it is trying to import that too.

Test Plan

Test links

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

Related issues/PRs

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jan 24, 2025
Copy link

netlify bot commented Jan 24, 2025

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 407601d
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/6794252f72b03800080b71db
😎 Deploy Preview https://deploy-preview-10866--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.

Copy link

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO Report
/ 🟠 63 🟢 98 🟢 96 🟢 100 Report
/docs/installation 🟠 50 🟢 97 🟢 100 🟢 100 Report
/docs/category/getting-started 🟠 72 🟢 100 🟢 100 🟠 86 Report
/blog 🟠 62 🟢 96 🟢 100 🟠 86 Report
/blog/preparing-your-site-for-docusaurus-v3 🟠 62 🟢 92 🟢 100 🟢 100 Report
/blog/tags/release 🟠 63 🟢 96 🟢 100 🟠 86 Report
/blog/tags 🟠 73 🟢 100 🟢 100 🟠 86 Report

@slorber slorber added the Argos Add this label to run UI visual regression tests. See argos.yml GH action. label Jan 30, 2025
Copy link

argos-ci bot commented Jan 30, 2025

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) ✅ No changes detected - Jan 30, 2025, 5:23 PM

@slorber slorber changed the title fix(theme): for feeds, don’t include code block buttons outside of br… fix(theme): Hide code block buttons before React hydration Jan 30, 2025
@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Jan 30, 2025
Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

Going to approve that considering those buttons require React hydration and do not work without JS at all.

More important than the RSS feeds, this decrease the static HTML we render for all sites using code blocks, as seen in our CI job here: https://github.com/facebook/docusaurus/actions/runs/12959468429/job/36431423219?pr=10866

CleanShot 2025-01-30 at 18 11 14

The impact is quite significant due to our usage of SVG React components (a practice we should change in #5865)

I'm not sure this will solve all your RSS to email issues (btw I'm curious to see that in action, or at least a screenshot), but it feels like a good change for Docusaurus anyway.

Related issues worth mentioning regarding code block buttons position/visibility:

@slorber
Copy link
Collaborator

slorber commented Jan 30, 2025

I think for blog RSS feeds, we should expose a feature that allows you to process the blog HTML that we use to generate the feed.

We have a createFeedItems API: https://docusaurus.io/docs/next/api/plugins/@docusaurus/plugin-content-blog#feedOptions.createFeedItems

But it's not super flexible for this case: you can override all/nothing, and cannot really do things such as filtering a few HTML tags before creating the feed.

Our current logic looks like this, and I think we could offer you a convenient way to add your own cheerio processing eventually.

      const content = await readOutputHTMLFile(
        permalink.replace(baseUrl, ''),
        outDir,
        trailingSlash,
      );

      const $ = cheerioLoad(content);

      const blogPostAbsoluteUrl = applyTrailingSlash(
        normalizeUrl([siteUrl, permalink]),
        {
          trailingSlash,
          baseUrl,
        },
      );

      const toAbsoluteUrl = (src: string) =>
        String(new URL(src, blogPostAbsoluteUrl));

      // Make links and image urls absolute
      // See https://github.com/facebook/docusaurus/issues/9136
      $(`div#${blogPostContainerID} a, div#${blogPostContainerID} img`).each(
        (_, elm) => {
          if (elm.tagName === 'a') {
            const {href} = elm.attribs;
            if (href) {
              elm.attribs.href = toAbsoluteUrl(href);
            }
          } else if (elm.tagName === 'img') {
            const {src, srcset: srcsetAttr} = elm.attribs;
            if (src) {
              elm.attribs.src = toAbsoluteUrl(src);
            }
            if (srcsetAttr) {
              elm.attribs.srcset = srcset.stringify(
                srcset.parse(srcsetAttr).map((props) => ({
                  ...props,
                  url: toAbsoluteUrl(props.url),
                })),
              );
            }
          }
        },
      );

      const feedContent = $(`#${blogPostContainerID}`).html()!,

@slorber slorber merged commit 78f44d0 into facebook:main Jan 30, 2025
40 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Argos Add this label to run UI visual regression tests. See argos.yml GH action. CLA Signed Signed Facebook CLA pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants