Skip to content

fix(website): ensure feedback widget renders with correct theme#11224

Merged
slorber merged 3 commits intofacebook:mainfrom
p-m-p:main
Jun 2, 2025
Merged

fix(website): ensure feedback widget renders with correct theme#11224
slorber merged 3 commits intofacebook:mainfrom
p-m-p:main

Conversation

@p-m-p
Copy link
Copy Markdown
Contributor

@p-m-p p-m-p commented Jun 1, 2025

If a user lands on the feature request community page with the theme set to dark the canny widget is rendered twice, first light and then dark. The first call with the light theme takes effect as the widget has not yet loaded and the widget shows in light theme. Adding a small delay prevents the first render with the wrong theme.

CleanShot 2025-06-01 at 07 40 20@2x

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.

Test links

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

If a user lands on the feature request community page with the theme set to dark the canny
widget is rendered twice, first light and then dark. The first call with the light theme takes
effect as the widget has not yet loaded and the widget shows in light theme. Adding a small
delay prevents the first render with the wrong theme.
@p-m-p p-m-p requested review from Josh-Cena and slorber as code owners June 1, 2025 06:45
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Jun 1, 2025
@netlify
Copy link
Copy Markdown

netlify bot commented Jun 1, 2025

[V2]

Name Link
🔨 Latest commit 5081557
🔍 Latest deploy log https://app.netlify.com/projects/docusaurus-2/deploys/683d78fd7a078d000888131b
😎 Deploy Preview https://deploy-preview-11224--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 project configuration.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jun 1, 2025

⚡️ Lighthouse report for the deploy preview of this PR

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

@slorber
Copy link
Copy Markdown
Collaborator

slorber commented Jun 2, 2025

Hey thanks for noticing this little bug

I'm not a fan of setTimeout with magic values, so I changed the fix for a better implementation.


Although both solutions work and fix the issue, the root source of the error is due to useColorMode(), which is a quite misleading historical hook we have.

On first render, it always returns the "default" theme we use during SSG to avoid React hydration mismatches. And the ColorModeProvider is applied by the Layout component, which resets the color mode state when you navigate to this page.

TLDR, during both initial load (hydration) and navigation, the useColorMode() always returns initially "light" (even if you are effectively in dark mode), and then eventually switch to "black". The canny problem only appears for the initial load by chance, but in both cases, canny is called first with "light" and then called with "dark". We should ensure that at most, canny is initialized only once.

We should find a way to fix the root cause, but it likely requires a breaking change to the publicly documented useColorMode() hook, so I noted to take another look at this for v4. Until then, fixing the symptom (and not the cause) is good enough.

@slorber slorber added the pr: ignore This PR is not meaningful enough to appear in the changelog. label Jun 2, 2025
@slorber slorber merged commit dacfc17 into facebook:main Jun 2, 2025
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed Signed Facebook CLA pr: ignore This PR is not meaningful enough to appear in the changelog.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants