-
Notifications
You must be signed in to change notification settings - Fork 45
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
Code editor additional languages #2684
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git βοΈ
|
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.
Tested it out and looked good to me, just a couple small nits.
Ruby = 'ruby', | ||
Shell = 'shell', | ||
Go = 'go', | ||
Hcl = 'hcl', | ||
Javascript = 'javascript', |
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.
Nit: should it be JavaScript
?
"@hashicorp/design-system-components": minor | ||
--- | ||
|
||
hds-code-editor: Add language syntax highlighting suport for JavaScript and Rego |
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.
Nit: should we include that theres a new dep @codemirror/lang-javascript
?
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.
Looks great! Just a couple small comments
console.log(message); | ||
} | ||
|
||
sayMessage(); | ||
}`, |
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.
"@hashicorp/design-system-components": minor | ||
--- | ||
|
||
hds-code-editor: Add language syntax highlighting suport for JavaScript and Rego |
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.
hds-code-editor: Add language syntax highlighting suport for JavaScript and Rego | |
`hds-code-editor` modifier - Add language syntax highlighting suport for JavaScript and Rego | |
`CodeEditor` - Add language syntax highlighting suport for JavaScript and Rego |
[nit] Need to tweak format slightly, and add in a line for the CodeEditor
so the parser generate a version history for it.
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.
There aren't any direct changes to the code editor component, just the modifier. Do you think we still need to tag the component with changes?
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.
There aren't any direct changes to the code editor component, just the modifier. Do you think we still need to tag the component with changes?
While your take is accurate, all instances using the modifier (in our case the CodeEditor
component) are benefitting from this new feature, so surfacing it at the component level makes sense to me. While we offer both a component and a modifier, the latter is more of an escape hatch for bespoke implementations.
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.
Makes sense - Thanks for your clarification.
Design will have to add these to the CodeEditor ~ will open up a branch for this in Figma. |
π Summary
If merged, this PR adds syntax highlighting support for JavaScript and Rego to the
hds-code-editor
modifier.π οΈ Detailed description
πΈ Screenshots
π External links
Jira tickets:
HDS-4452
HDS-4469
π Component checklist
π¬ Please consider using conventional comments when reviewing this PR.