-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Add discourse comment feature #2433
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
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ba6576b to
e96e742
Compare
alexandratran
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To clarify, this PR will add a discourse comments section underneath each tutorial page, without affecting other parts of the UX? And this can't be previewed, only viewed once merged into production?
One comment I have is I'm not a fan of including the tutorial cover image at the top of each page. The text in the cover is the same as the already displayed title in most cases, so it feels redundant and takes up a lot of space.
| } | ||
|
|
||
| initializeEmbed() | ||
| }, [postUrl, discourseTopicId, DISCOURSE_API_KEY]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Stale Closures from Incomplete Effect Dependencies
The useEffect dependency array is incomplete. The effect uses several variables that are not included in the dependencies: DISCOURSE_API_USERNAME, DISCOURSE_CATEGORY_ID, and metadata. Additionally, the functions findDiscussionTopic, loadCleanEmbed, searchExistingTopic, and normalizeEmbedUrl are defined in the component body and reference these variables, but React's exhaustive-deps rule would flag this. This can cause stale closures where the effect uses old values of these variables when they change, leading to incorrect API calls or search logic.
…sk/metamask-docs into feat/tutorial-builder-hub
| date, | ||
| wrapperClassName, | ||
| communityPortalTopicId, | ||
| discourse_topic_id, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Domain Mismatch Breaks Social Sharing
The url variable is constructed with https://metamask.io domain, but the site actually runs on https://docs.metamask.io. This causes social sharing links (Facebook, Twitter) and the copy-to-clipboard feature to generate incorrect URLs pointing to metamask.io instead of docs.metamask.io, breaking the sharing functionality for tutorial pages.

Add discourse comment feature in Tutorials for syncing with Builder Hub
Note
Integrates Discourse comments into tutorial pages with topic lookup/embedding and teaser rendering, adds per-tutorial topic IDs, and enriches Open Graph image metadata.
customFieldsindocusaurus.config.js.src/components/DiscourseCommentto normalize URLs, look up topics via Discourse API, load clean embeds, and render optional teaser content.src/theme/MDXPageto extract first-paragraph teasers from MDX, pass page metadata/topic ID toDiscourseComment, and use absolute cover image URLs.discourseTopicIdfrontmatter to multiplesrc/pages/tutorials/*files to bind pages to forum topics.src/components/SEO(secure URL, type, dimensions, alt).Written by Cursor Bugbot for commit ce89d44. This will update automatically on new commits. Configure here.