feat: add 'Open in Playground' button to learn modules#877
feat: add 'Open in Playground' button to learn modules#877Atharva7115 wants to merge 2 commits intoaccordproject:mainfrom
Conversation
✅ Deploy Preview for ap-template-playground ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Signed-off-by: Atharva Chandurkar <atharvachandurkar7115@gmail.com>
Atharva7115
force-pushed
the
feat/open-playground-button
branch
from
f35f849 to
239f463
Compare
There was a problem hiding this comment.
Pull request overview
Adds a learn → playground bridge by introducing an “Open in Playground” button on learning modules and supporting ?sample=... URL initialization so the Playground can auto-load the relevant sample template.
Changes:
- Teach the store initializer to load a sample when
?sample=...is present. - Add
sampleNamemappings to learning steps for each module. - Render an “Open in Playground” button in learning module content that navigates to
/?sample=....
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/store/store.ts | Adds support for initializing the app from a sample query param by calling loadSample. |
| src/constants/learningSteps/steps.ts | Associates learn modules with sample names to drive the new navigation behavior. |
| src/components/Content.tsx | Adds the “Open in Playground” CTA to learning module pages and wires it to navigation. |
| init: async () => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| const compressedData = params.get("data"); | ||
| const sampleName = params.get("sample"); | ||
| if (compressedData) { | ||
| await get().loadFromLink(compressedData); | ||
| } else if (sampleName) { | ||
| await get().loadSample(sampleName); | ||
| } else { | ||
| await get().rebuild(); | ||
| } |
There was a problem hiding this comment.
init() now calls loadSample(sampleName) when ?sample=... is present, but loadSample is a no-op when the sample name is unknown. In that case init() will skip rebuild() and the app can remain with agreementHtml unset/empty. Consider making loadSample throw/return a boolean when the sample is not found, and have init() fall back to rebuild() (and/or surface an error) when the sample cannot be loaded.
| init: async () => { | ||
| const params = new URLSearchParams(window.location.search); | ||
| const compressedData = params.get("data"); | ||
| const sampleName = params.get("sample"); | ||
| if (compressedData) { | ||
| await get().loadFromLink(compressedData); | ||
| } else if (sampleName) { | ||
| await get().loadSample(sampleName); | ||
| } else { | ||
| await get().rebuild(); | ||
| } |
There was a problem hiding this comment.
New behavior (loading a sample via ?sample=... in init()) is user-facing and affects initialization flow, but there’s no unit/integration test covering it. Please add a store-level test that sets window.location.search to include sample and asserts loadSample is invoked (or sampleName/editor values update) and agreementHtml is produced after initialization.
| { title: "Module 1", link: "/learn/module1", sampleName: "Hello World" }, | ||
| { title: "Module 2", link: "/learn/module2", sampleName: "Formula Now" }, | ||
| { title: "Module 3", link: "/learn/module3", sampleName: "Join" }, |
There was a problem hiding this comment.
sampleName values are hardcoded strings that must exactly match Sample.NAME values for loadSample to work. This duplicates source-of-truth data and can silently break if sample names change. Consider importing the NAME constants from the relevant src/samples/* modules (or exporting a shared sample ID enum) and referencing those here instead of repeating the strings.
| <div style={{ display: "flex", justifyContent: "flex-end", marginBottom: "12px", gap: "10px" }}> | ||
| {steps[currentIndex]?.sampleName && ( | ||
| <Button | ||
| type="primary" | ||
| onClick={() => navigate(`/?sample=${encodeURIComponent(steps[currentIndex].sampleName!)}`)} | ||
| style={{ | ||
| backgroundColor: colors.primary, | ||
| borderColor: colors.primary, | ||
| fontSize: "0.9rem", | ||
| height: "auto", | ||
| padding: "4px 12px" | ||
| }} | ||
| > | ||
| Open in Playground | ||
| </Button> |
There was a problem hiding this comment.
The new "Open in Playground" button introduces a new learn→playground navigation path, but there’s no component test covering that the button renders for modules with a sampleName and that clicking it navigates to /?sample=.... Given existing component tests in src/tests/components, adding a test for this behavior would help prevent regressions.
|
@copilot apply changes based on the comments in this thread |
|
@copilot apply changes based on the comments in this thread |
|
Thanks for this contribution! This appears to be a duplicate of #872 by @karanpatill, which was opened earlier and addresses the same issue (#871). @Atharva7115 — would you mind helping review #872? Your input would be valuable since you've worked on the same feature. This comment was generated by AI on behalf of @mttrbrts. |
feat(playground): Add "Open in Playground" button to learning modules
Closes #871
Changes
?sample=...)loadSamplelogic from storesampleNamemapping insteps.tsfor each modulecolors.primary) instead of hardcoded valuesFlags
Screenshots or Video
Preview link :
Related Issues
Author Checklist
--signoffoptionmainfromfork:branchnameNotes
This implementation uses the existing
colors.primarytheme token instead of hardcoded color values.