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

fix: ProseCode class attribute warning #2121

Closed
wants to merge 2 commits into from
Closed

Conversation

bobby169
Copy link

#2111

πŸ”— Linked issue

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@netlify
Copy link

netlify bot commented Jun 26, 2023

βœ… Deploy Preview for nuxt-content canceled.

Built without sensitive environment variables

Name Link
πŸ”¨ Latest commit a0e1fa2
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt-content/deploys/649aa4fd3c953200089740f0

@narasimhajupally
Copy link

narasimhajupally commented Jun 26, 2023

we can just wrap the slot in <div> making it root node, doing so the class prop will be automatically inherited.

render a single root node the class prop will be automatically inherited
@farnabaz
Copy link
Member

Thanks for the PR.
Instead of wrapping code, a better solution would be moving class attribute from component itself into inner code tag ( pre > code)

from:

className: [`language-${language}`]
},
[h(node, 'pre', {}, [h(node, 'code', { __ignoreMap: '' }, [u('text', code)])])]

to:

[h(node, 'pre', {}, [h(node, 'code', { __ignoreMap: '', className: [`language-${language}`] }, [u('text', code)])])]

@wJoenn
Copy link

wJoenn commented Jul 12, 2023

@farnabaz I just tried your solution locally and it doesn't solve Vue's warning.
Bobby's fix by wrapping the slot inside a div does on the other hand.

Wouldn't it be just fine to merge this as is ?

It could even serve a second purpose by adding a span with the filename to be able to display the filename in the corner of the code block

<template>
  <div class="codeBlock">
    <span class="filename">{{ filename }}</span>
    <slot />
  </div>
</template>

<style>
.codeBlock {
  position: relative;
}

.codeBlock .filename {
  position: absolute;
  right: 20px;
  top: 10px;
}

.codeBlock pre code .line {
  display: block;
  min-height: 1rem;
}
</style>

image

@farnabaz
Copy link
Member

Having filename indeed is a good thing to have, but also some user may not want to have a wrapper tag.
As I checked a couple of websites, they add the class into pre tag. Check https://shiki.matsu.io/

Also, I've checked it locally, adding class to pre tag resolves the warning πŸ™‚

@farnabaz
Copy link
Member

I'm closing this in favor of #2187.
This issue is fixed in nuxt-mdc package

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants