Website: actually render GTS code snippets in code previews#3944
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR updates the website docs “code group” rendering pipeline so that when a .gts tab is selected, the preview renders the actual .gts demo component module (instead of always rendering the .hbs version). This improves accuracy of docs previews for modern GTS examples.
Changes:
- Add a
gtsFilenameforward-prop from markdown demo processing through the Showdown extension into<Doc::CodeGroup>. - Update
<Doc::CodeGroup>to choose between runtime-compiled.hbspreviews and module-rendered.gtspreviews based on the active tab. - Ensure
.gtscode-snippet demo modules are included in the app build tree so they can be rendered.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| website/lib/markdown/markdown-process-demos.js | Adds forwarding of a separate gtsFilename for demo-block previews. |
| website/lib/markdown/index.js | Includes code-snippets/**/*.gts in the app tree so demos can render. |
| website/docs/components/accordion/partials/code/code-snippets/accordion-size.gts | Temporarily changes displayed text to visually confirm .gts preview rendering. |
| website/app/shared/showdown-extensions/content-blocks.js | Updates demo-block parsing to accept and pass @gtsFilename into <Doc::CodeGroup>. |
| website/app/components/dynamic-template.gjs | Refactors caching and supports rendering precompiled component modules via component id. |
| website/app/components/doc/code-group/index.gts | Switches preview rendering logic to use .gts component id when the .gts tab is active. |
didoo
left a comment
There was a problem hiding this comment.
I have done a high-level review. My instinct tells me there is space for simplification of the code, expecially around the way in which we handle the different use cases/conditions, and for some clean-up too (naming things more consistently/semantically, adding comments to blocks of code that are not "obvious", etc)
If you want I can jump on a call, and we can discuss more in detail what I mean.
| }; | ||
|
|
||
| let fileNameToForward = ''; | ||
| let gtsFileNameToForward = ''; |
There was a problem hiding this comment.
I have tried to understand, with no luck, if it's really necessary to have a distinct gtsFileNameToForward or if instead we could simply reuse the fileNameToForward instead (we know the convention is to have the files to use the same name, only different extensions) and with that simplify the logic.
If so, can you convince/explain me why? And in that case, should fileNameToForward be renamed to something like classicFileNameToForward?
also, nit: in some case it's spelled filename (lowecase n) in other FileName (uppercase N), maybe better to use the same convention?
There was a problem hiding this comment.
I fixed it so it name always does "filename" instead of "fileName"
as for passing both, I prefer to keep passing both to keep all the file name parsing/making assumptions about the file extensions in this file. previously weve been really explicit about passing the demo information to the code group and Id like to keep that pattern (ex. we could parse the compact gts code snippet in the code group, but opt to do it here instead).
As for renaming filename, sometimes it is the gts file name (like in cases where there is only a gts example - this happens on the getting started for engineers page). in the future when we dont support both anymore, id imagine wed just make it so filename is always the gts one and remove gtsFilename.
There was a problem hiding this comment.
ok after thinking about it more, I made some more changes on the last point. now I just explicitly pass 'gtsFilename' and 'classicFilename' to make it as clear as possible.
There was a problem hiding this comment.
I left a few comments, but sorry to insist I still think there is work to do in terms of reducing the overhead in logic branching and getter. So, sorry if I copy verbatim what Copilot confirmed when asked, but 1) it express the issue more clearly of what I could do and 2) it confirms what my senses are telling me, which is there are too many abstractions/indirections/getters for something that probably could be much simpler/linear:
It also added this comments, which I think make sense:
|
@didoo I have simplified the logic the way you described, I def agree that it is much clearer now. |
didoo
left a comment
There was a problem hiding this comment.
I have left a few more comments. Plus, I am going to send you a diff to show how a single filename (well, componentId) for both hbs and gts would work (not a hill I am going to die for, but I think simplifies/rationalises a bit the code).
One other thing I noticed is that in the local environment I see this messages in the console (not in the deployed preview). Something we should "fix"?
There was a problem hiding this comment.
[todo] a couple of changes to this regex:
- the
.jsextension is not needed (getComponentIdFromPathis never used for.jsfiles`) - using
.classic.hbsinstead of.hbsleads to the two filenames be the same forgtsandhbs
| /((?:getting-started|foundations|components|layouts|utilities|overrides|patterns|testing).*?)\.(?:classic\.hbs|gts)/; |
didoo
left a comment
There was a problem hiding this comment.
Left a few non-blocking comments. Up to you if you want to address them or not. In any case I am approving, the code is much better now, thanks. 👍


📌 Summary
Currently, in the code group component - the preview is only ever the hbs version of the snippet. When you are on the gts snippet, it still renders the hbs version. This PR updates the component to actually render the gts component when the gts tab is active.
Currently, there is a difference in the Accordion size demo so it is easier to verify the new behavior, this will be reverted before the PR merges.
PREVIEW
📸 Screenshots
Before

After

🔗 External links
Jira ticket: HDS-5833
💬 Please consider using conventional comments when reviewing this PR.
📋 PCI review checklist
Examples of changes to controls include access controls, encryption, logging, etc.
Examples include changes to operating systems, ports, protocols, services, cryptography-related components, PII processing code, etc.