Skip to content
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

feat: Support code group in Markdown #1242

Closed
wants to merge 13 commits into from
Closed

Conversation

VoVAllen
Copy link
Contributor

@VoVAllen VoVAllen commented Aug 26, 2022

Signed-off-by: Jinjing.Zhou allenzhou@tensorchord.ai

fixes #728

I made a new page to test it and found I still need to manually import the SFC. How can I enable this SFC by default? @brc-dd

Need suggestion on the color pattern

Change: Originally the upper-right side will show the code language. Now it will show title specified by user, if not will be the language itself.

demo: https://deploy-preview-1242--vitepress-docs.netlify.app/guide/test

Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
@Sepush
Copy link

Sepush commented Aug 26, 2022

this feature pretty cool

@rainbowatcher
Copy link

it's looks like doesn't work well on mobile

@brc-dd brc-dd marked this pull request as draft August 26, 2022 15:37
VoVAllen and others added 2 commits August 27, 2022 01:32
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
@VoVAllen VoVAllen marked this pull request as ready for review August 26, 2022 17:34
@VoVAllen VoVAllen marked this pull request as draft August 26, 2022 17:34
@VoVAllen VoVAllen requested a review from Sepush August 26, 2022 17:34
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
@VoVAllen VoVAllen marked this pull request as ready for review August 27, 2022 08:22
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
@VoVAllen
Copy link
Contributor Author

VoVAllen commented Aug 27, 2022

The current color pattern doesn't seem good. Any suggestion is welcomed

@meteorlxy
Copy link
Member

meteorlxy commented Aug 30, 2022

As code-group is a theme-specific feature, I wonder if we should put it into the core, including:

  • The markdown container plugin
  • The markdown preWrapper plugin

An ideal way is to only support it in the default theme. 🤔

@VoVAllen
Copy link
Contributor Author

@meteorlxy The prewrapper change should work for all themes. Is it fine to keep it for all themes?

For the container part, does it break anything if that is not implemented by other themes? (transform ::code-group in to <CodeGroup></CodeGroup>)

@VoVAllen
Copy link
Contributor Author

VoVAllen commented Sep 4, 2022

Ping @meteorlxy @brc-dd

@brc-dd
Copy link
Member

brc-dd commented Sep 4, 2022

For the container part, does it break anything if that is not implemented by other themes? (transform ::code-group in to <CodeGroup></CodeGroup>)

Not unless they are using ::code-group in their markdown files, in that case they would need to register a global component CodeGroup.

Just a thought - this likely can be done in pure HTML/CSS without need of any Vue component. (make md plugin render something like radio input + pre, then use :checked). I'll give that a try, otherwise we'd go ahead with this PR.

@VoVAllen
Copy link
Contributor Author

VoVAllen commented Sep 4, 2022

@brc-dd I see what you mean (like https://codepen.io/xboxyan/pen/oMxmxm). It seems viable in this scenario, with lots of tricks in CSS selector. Seems dynamic CSS generation is needed here. Is this a good option?

@brc-dd
Copy link
Member

brc-dd commented Sep 4, 2022

Seems dynamic CSS generation is needed here.

Not quite. In your example too the CSS is static (its independent of those tab IDs).

Is this a good option?

Perhaps. I'm not sure yet. This would prevent the need of defining a custom component, so this will be helpful to developers writing a custom theme. It would sort of become a markdown feature instead of theme one (they can customize using CSS vars). Also, there are already certain markdown-it plugins that do that, so it wouldn't be hard to implement. My only concern is with a11y. That would need investigation.

@VoVAllen
Copy link
Contributor Author

VoVAllen commented Sep 4, 2022

Actually I think we can avoid using SFC, but doesn't have to be pure CSS. We can still use js for simplicity. WDYT? @brc-dd

@brc-dd
Copy link
Member

brc-dd commented Sep 4, 2022

Yeah that would work too. We are injecting JS for copy code stuff too so that it works across custom themes.

Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
Signed-off-by: Jinjing.Zhou <allenzhou@tensorchord.ai>
@VoVAllen
Copy link
Contributor Author

VoVAllen commented Sep 18, 2022

I've updated the code to implement this without SFC. Please take a look @brc-dd . Thanks!
demo: https://deploy-preview-1242--vitepress-docs.netlify.app/guide/test

@VoVAllen
Copy link
Contributor Author

@brc-dd Does current implementation work? If so I can fix the docs so that we can merge it

@VoVAllen
Copy link
Contributor Author

Also suggestion on color pattern is welcomed

@brc-dd
Copy link
Member

brc-dd commented Oct 14, 2022

Does current implementation work?

Yeah, I'm working on some refinements. Hopefully, will get this merged this weekend (month 😓).

@brc-dd brc-dd mentioned this pull request Oct 30, 2022
3 tasks
@brc-dd
Copy link
Member

brc-dd commented Oct 30, 2022

Can you guys review #1560 and see if there are any issues (especially with UI)? Here is the deploy preview: https://deploy-preview-1560--vitepress-docs.netlify.app/test

@kiaking kiaking closed this in a684b67 Dec 16, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support code groups and allow adding title to code blocks
5 participants