Website: add support for gts/hbs code snippets #3389
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
shleewhite
changed the title
|
|
||
| # temporary ignore | ||
| /docs/ | ||
| /docs/**/*.md |
There was a problem hiding this comment.
I made the prettier ignore more specific so the code snippets would be fixed by prettier.
| "ember-router-scroll": "^4.1.2", | ||
| "ember-shiki": "^0.3.0", | ||
| "ember-source": "~6.4.0", | ||
| "ember-style-modifier": "^4.4.0", |
There was a problem hiding this comment.
Added the style modifier because examples use it but it was not previously a dependency of the website.
shleewhite
force-pushed
the
SPIKE/website-code-snippets
branch
from
40f603c to
1ea4653
Compare
shleewhite
force-pushed
the
SPIKE/website-code-snippets
branch
from
b0ec28a to
07cea48
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces a new code group component that displays both HBS and GTS code snippets side by side, enhancing the documentation experience by allowing users to view component examples in multiple formats. The implementation includes a custom build step to process demo blocks in markdown files, replacing the deprecated ember-cli-clipboard with a custom clipboard modifier, and adding new dependencies like ember-shiki for syntax highlighting.
Key changes:
- New
Doc::CodeGroupcomponent for displaying HBS/GTS code snippets with language switching - Custom build pipeline step (
markdown-process-demos.js) to transform[[demo:]]markdown directives into CodeGroup components - Custom
doc-clipboardmodifier to replaceember-cli-clipboardfor v2 addon compatibility
Reviewed changes
Copilot reviewed 90 out of 91 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| website/lib/markdown/markdown-process-demos.js | New build step that processes demo blocks in markdown and extracts code from HBS/GTS/JS files |
| website/lib/markdown/index.js | Integrates the new demo processing step into the markdown build pipeline |
| website/app/components/doc/code-group/index.gts | New component for displaying code snippets with language switching and preview |
| website/app/styles/doc-components/code-group.scss | Styling for the code group component |
| website/app/modifiers/doc-clipboard.ts | Custom clipboard modifier to replace ember-cli-clipboard |
| website/app/components/doc/copy-button/index.gts | Refactored to use custom clipboard modifier instead of ember-cli-clipboard |
| website/config/fastboot.js | Fastboot configuration for ember-shiki compatibility |
| website/docs/components/button/partials/code/how-to-use.md | Converted from inline code blocks to demo directives |
| website/docs/components/button/partials/code/code-snippets/*.hbs | New HBS code snippet files for button examples |
| website/docs/components/button/partials/code/code-snippets/*-component.gts | New GTS code snippet files for button examples |
| website/docs/components/badge/partials/code/how-to-use.md | Converted from inline code blocks to demo directives |
| website/docs/components/badge/partials/code/code-snippets/*.hbs | New HBS code snippet files for badge examples |
| website/docs/components/badge/partials/code/code-snippets/*-component.gts | New GTS code snippet files for badge examples |
| website/docs/components/accordion/partials/code/how-to-use.md | Converted from inline code blocks to demo directives |
| website/docs/components/accordion/partials/code/code-snippets/*.hbs | New HBS code snippet files for accordion examples |
| website/docs/components/accordion/partials/code/code-snippets/*-component.gts | New GTS code snippet files for accordion examples |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
website/docs/components/button/partials/code/code-snippets/button-icon-only-component.gts
Outdated
Show resolved
Hide resolved
website/docs/components/button/partials/code/code-snippets/button-loading.js
Show resolved
Hide resolved
jorytindall
left a comment
jorytindall
left a comment
There was a problem hiding this comment.
Couple of small nit-picky comments, but this is looking great!
| background-color: var(--doc-color-feedback-information-300); | ||
| } | ||
|
|
||
| &:active { |
There was a problem hiding this comment.
Technically I had to slightly alter the color to #0033B0 instead of information-100 to increase the contrast for this. But if it isn't an issue for the interactive state, I'll gladly change it back to the token.
There was a problem hiding this comment.
@jorytindall Is this something we should change more holistically?
There was a problem hiding this comment.
whatever we decide, can we use (or define) --doc CSS variables instead of hex values? it will make theming (if needed) easier in the future
| } | ||
|
|
||
| .doc-code-group__expand-button { | ||
| @include doc-font-style-code(); |
There was a problem hiding this comment.
I'm not sure what the overlap in tokens is between the website code and Figma, but I'm using code/medium size which is 13px (this renders 14px)
There was a problem hiding this comment.
interesting! should I change that for the whole website? its set here:
|
|
||
| const AccordionA11yName: TemplateOnlyComponent<AccordionA11yNameSignature> = | ||
| <template> | ||
| <HdsAccordion ...attributes as |A|> |
There was a problem hiding this comment.
why do we need ...attributes in the .gts examples? also, this may cause confusion for the consumers ("why hbs and gts are different?"), so probably we should try to keep them as aligned as possible (also for our own future sanity: we will never remember if some differences are intentional/needed or not)
| } from '@hashicorp/design-system-components/components'; | ||
| import type { HdsAccordionSignature } from '@hashicorp/design-system-components/components/hds/accordion/index'; | ||
|
|
||
| interface AccordionContainsInteractive2Signature { |
There was a problem hiding this comment.
can we think of ways to avoid using progressive numbers? what if in the future we need to add an example between 1 and 2? we would have to rename one of the files, which is not a big problem per se, but still...
There was a problem hiding this comment.
Something to keep in mind: extra space at the bottom renders as extra space in the code snippet:
|
|
||
| let processedDocsJsonFilesTree = new MarkdownToJson( | ||
| processedDocsMardownFilesTree, | ||
| processedDocsMarkdownFilesTree, |
| "website/app/routes/show.js", | ||
| "website/app/controllers/show.js", | ||
| "website/app/templates/show.hbs", | ||
| # Don't include header on examples for the website |
| const match = this.unescapedGtsSnippet.match(templateRegex); | ||
| if (!match?.[1]) { | ||
| return ''; |
There was a problem hiding this comment.
should we notify the "user" (us) too? something like a console.log|error? or simply a message instead (like it's done with the Unable to load file message that appears inside the CodeGroup?)
| const nonEmptyLines = lines.filter((line) => line.trim().length > 0); | ||
| if (nonEmptyLines.length === 0) { | ||
| return ''; |
There was a problem hiding this comment.
is this a real use case that needs to be accounted for? 🤔
| // Remove leading and trailing blank lines | ||
| snippet = snippet.replace(/^\s*\n/, '').replace(/\n\s*$/, ''); | ||
| // Find the minimum indentation level (excluding empty lines) |
There was a problem hiding this comment.
it's a lot of JS code to run to dedent the code (expecially for large pages, with a lot of large code snippets); if we see some performance issues, we can try to optimize things, but for now it's OK
There was a problem hiding this comment.
I have a question (suggestion?) though: since these code snippets are the ones that have already been partially processed, why not do these transformations upfront, in the markdown-process-demos.js files? do you think it would it be possible?
There was a problem hiding this comment.
and if it's because we want to "run/execute" the code, for the preview, maybe we could have one more extra argument, for the reindented gts code.
(I am really worried that this may impact the rendering of pages, think of the Table/AdvancedTable pages, which are already pretty slow to render)
| return this.currentView === 'hbs' | ||
| ? this.unescapedHbsSnippet | ||
| : this.gtsSnippet; |
There was a problem hiding this comment.
too much?
| return this.currentView === 'hbs' | |
| ? this.unescapedHbsSnippet | |
| : this.gtsSnippet; | |
| return this.currentView === 'hbs' | |
| ? this.unescapedHbsSnippet | |
| : this.isExpanded | |
| ? this.unescapedGtsSnippet | |
| : this.shortenedGtsSnippet; |
| : this.gtsSnippet; | ||
| } | ||
| get copyButtonLabel() { |
There was a problem hiding this comment.
a lot of this logic already exists in the DocCopyButton, do you think there is a way in which we could re-use that component, instead of creating an almost identical copy?
5 participants
📌 Summary
If merged, this PR would create a new code group component that displays both hbs and gts code snippets.
To see the new code groups in action:
Changes include:
ember-cli-clipboardto a custom clipboard modifier based on the hdsClipboard. This is becauseember-cli-clipboardis not a v2 addon and can't work ingts.[[demo: code-snippets/filename.hbs]]into the correct componentPotential follow ups:
🛠️ Detailed description
To make this new component, I added a new step to the website build process. These are the steps to add a new demo to a page:
1. Create a new code snippet, one in hbs and one in gts.
Note: the gts version needs to have the same name as the hbs file, except add
-componentto the end. This is because if they have the same exact name, ember thinks both files are part of the same component and the types get messed up.So for example, the new files would be
button-basic.hbsandbutton-basic-component.gtsIf the example needs additional javascript, just add a js file as well. In the previous example, it would be
basic-button.js.2. Add the new demo to the page in markdown
In the markdown, add a demo and pass the relative path to the handlebars file.
In the example, it woudl be:
[[demo: code-snippets/button-basic.hbs]]If you don't want the code to execute, simply do
[[demo: code-snippets/button-basic.hbs execute=false]].📸 Screenshots
Before
After
🔗 External links
Jira ticket: HDS-5316
Figma file: https://www.figma.com/design/Ky0qWjvHZR3je1lCBlzNB1/HDS-Website--Initial-Build-?node-id=5827-16315&t=BAV6D7DOHG0WIYFY-0
💬 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.