feat(codebox): adding possibility to switch between lang#3249
feat(codebox): adding possibility to switch between lang#3249ovflowd merged 21 commits intonodejs:mainfrom AugustinMauroy:refractor(blockCode)
Conversation
|
Need help:
|
There was a problem hiding this comment.
Quick Note: This PR should only land after the changes in the generator are done so that if any change happens there, this PR can be updated accordingly.
I see a few fundamental logic problems in this PR, such as how you're handling switching from another language and how you store the current language. Here's my suggestion 🙂
- From the go, you should have "languageOptions"; this is what you should store from the
matches?.groups?.lang - You should only have a state for
langIndex - Getting the current language and current label should come from
languageOptions[langIndex]
Last, I believe you're in the right direction, but still you need to revise a little bit your logic here :)
src/components/ArticleComponents/Codebox/__tests__/__snapshots__/index.test.tsx.snap
Show resolved
Hide resolved
Codecov ReportPatch coverage has no change and project coverage change:
Additional details and impacted files@@ Coverage Diff @@
## main #3249 +/- ##
==========================================
- Coverage 66.02% 65.53% -0.49%
==========================================
Files 118 145 +27
Lines 1351 1741 +390
Branches 342 407 +65
==========================================
+ Hits 892 1141 +249
- Misses 422 555 +133
- Partials 37 45 +8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Co-Authored-By: Claudio Wunder <[email protected]>
src/components/ArticleComponents/Codebox/__tests__/__snapshots__/index.test.tsx.snap
Show resolved
Hide resolved
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
An unrelated comment, can we update the interface Props to be something like:
type Props = PropsWithChildren<PropsWithChildren<{ className: string | null }>>
|
What do you think about allowing an |
github-actions
bot
removed
the
create-preview
|
Please find a preview at: https://staging.nodejs.dev/3249/ |
Co-authored-by: Shanmughapriyan S <[email protected]> Signed-off-by: Augustin Mauroy <[email protected]>
…roy/nodejs.dev into refractor(blockCode)
ovflowd
left a comment
ovflowd
left a comment
There was a problem hiding this comment.
This PR is ready. I would just ask to reply to my questions and remove the writing content doc from this PR and I think I can approve it :)
ovflowd
changed the title
ovflowd
changed the title
|
Superb work, @AugustinMauroy. It took a while, but we got to the right direction :) |
4 participants
Description
Adding possibility to switch between lang.
The label lang it's obligatory in lower case. It's more beautiful (personally) and it's better for dyslexia.
Modifying about content (```javascript -> ``js).
Todo list:
sync|asyncin "Lang" of code boxDemo:
code:
'''cjs|esm const { createServer } = require('http'); ------- import { createServer } from 'http'; '''Enregistrement.de.l.ecran.2023-03-05.a.12.15.33.mov
Feature unsupported
'''sync|async CODE sync ------- CODE async '''Related Issues
Related #2844 (feature of switching not for parsing of dock)