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

Isolated class names or Prefix class names for VitePress default theme #199

Closed
jd-solanki opened this issue Jan 4, 2021 · 27 comments · Fixed by #1104
Closed

Isolated class names or Prefix class names for VitePress default theme #199

jd-solanki opened this issue Jan 4, 2021 · 27 comments · Fixed by #1104
Labels
enhancement New feature or request

Comments

@jd-solanki
Copy link
Contributor

Is your feature request related to a problem? Please describe.
I am using VuePress and me generally create documentation for UI components which includes already created templates/components. In this case, when we import template styles it conflicts with VuePress's style.
e.g. container class is present in both styles (VuePress & Template/Project Styles) hence create undesired result.

Describe the solution you'd like
I guess prefixing class names will do the magic or some kind of isolation so it doesn't affect the user's classes/styles.

So here I am creating issue in VitePress so if possible this can be taken care of from start.

Describe alternatives you've considered
I created my own vuepress theme but it is not as flexible and stable as the default theme.

Additional context
None

@kiaking kiaking added the enhancement New feature or request label Feb 5, 2021
@kiaking
Copy link
Member

kiaking commented Feb 5, 2021

Thanks for the feedback! Good point. I was also thinking that we should "scope" styles. However, it might not be that super easy doing that inside markdown content.

Since the docs are generated from markdown, the tags will be generic <p> or <h1>, so even we scope the styles, it will affect any tags inside markdown content.

So in your case, if you were to add Vue Component inside markdown, it will get affected by those generic tag styles 🤔

I don't think it would make sense to add custom VitePress class to those generated tags like <p> tag. Because users may create custom markdown renderer that generates those generic tags, and in those cases, the style should be applied other wise users have to remember to add VitePress custom class to those custom tags...

That's being said, we should scope or even prefix generic "classes" such as container as you mentioned. Let's do that 💪

@kiaking
Copy link
Member

kiaking commented May 19, 2022

In next branch, the markdown styles are scoped under vp-doc class. Any generic element outside of Markdown has no default styles applied (we do normalize the elements using normalize.css though).

In order to stop affecting custom Vue Component rendered inside markdown, is another topic. This one is not that easy to solve.

We could add special class something like vp-raw, and reset all of the styles that are defined in vp-doc style... but that's kinda huge work 😅

It would be really cool if we have any neet idea to overcome this issue 🤔

@jd-solanki
Copy link
Contributor Author

Hi team 👋🏻

What if we add not psudo selector to vp-doc styles so that it doesn't apply styles if specific class is applied. Let's say if I apply not-vp-doc class to my demo container then it shouldn't add vp-doc styles. We can achieve this via below styles.

Change from:

.vp-doc h1,
.vp-doc h2,
.vp-doc h3,
.vp-doc h4,
.vp-doc h5,
.vp-doc h6 {
  position: relative;
  font-weight: 600;
  outline: none;
}

to

.vp-doc:not(.not-vp-doc) h1,
.vp-doc:not(.not-vp-doc) h2,
.vp-doc:not(.not-vp-doc) h3,
.vp-doc:not(.not-vp-doc) h4,
.vp-doc:not(.not-vp-doc) h5,
.vp-doc:not(.not-vp-doc) h6 {
  position: relative;
  font-weight: 600;
  outline: none;
}

Now if I have doc content like beow:


# Test

## This will get style

<div class="not-vp-doc">

## This won't get styles

</div>

it should work.

This is same as the typography preset of UnoCSS where not-prose will not style the content.


This will definetely resolve the issue where we need to isolate the vp-doc styles to content only (don't apply them to demo container).


Moreover, I suggest writing SCSS instead of CSS so we can use nesting and don't have to write :not on each line.

@kiaking
Copy link
Member

kiaking commented Jun 21, 2022

I like the idea. Maybe name it .vp-raw? (it's shorter and I think most users don't know what .vp-docs is).

Moreover, I suggest writing SCSS instead of CSS so we can use nesting and don't have to write :not on each line.

I never used SCSS so no, but, maybe it's time to consider adding postcss support with nesting plugin 😅

@jd-solanki
Copy link
Contributor Author

Do you want me to make a PR on this or have you already started working on it? @kiaking

@kiaking
Copy link
Member

kiaking commented Jun 21, 2022

Not yet! Please feel free to work on it 🙌

@jd-solanki
Copy link
Contributor Author

jd-solanki commented Jun 23, 2022

  1. Can I introduce SCSS formatter & linter stylelint?
  2. Do you mind if I add .vscode for a recommended extension.
  3. stylelint will also need .vscode for a few settings we can set to format on save
  4. I also suggest adding markdown linter for auto-formatting & linting

@jd-solanki
Copy link
Contributor Author

jd-solanki commented Jun 24, 2022

Hi @brc-dd

Sorry for ping but I am refactoring CSS to SCSS and I don't know what to do with src/client/theme-default/styles/fonts.css file.

Should I keep that file as it is (and just change the extension to scss) or I somehow remove the duplicate code, I don't have much experience with @font-face.

@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2022

Keep it as it is. But I don't think Kia wanted refactoring styles to SCSS.

@jd-solanki
Copy link
Contributor Author

I like the idea. Maybe name it .vp-raw? (it's shorter and I think most users don't know what .vp-docs is).

Moreover, I suggest writing SCSS instead of CSS so we can use nesting and don't have to write :not on each line.

I never used SCSS so no, but, maybe it's time to consider adding postcss support with nesting plugin sweat_smile

I asked him. Let me know if I should continue on it

@brc-dd
Copy link
Member

brc-dd commented Jun 24, 2022

@jd-solanki
Copy link
Contributor Author

778e294a406890bc49c93886dc2e5f2e

Oops.

Actually, I worked on it and I am almost there with this PR: #850

@jd-solanki
Copy link
Contributor Author

jd-solanki commented Jun 25, 2022

Regarding vp-raw:

(according to best of my knowledge and my research)

SCSS

It's tricky to add vp-raw to isolate styles. We still need to add :not psudo to all selectors.

However, if we can use PostCSS with SCSS (maybe this can help), we can modify the selectors somehow to automatically add :not psudo.

PostCSS

As said above we can automatically add :not psudo. But without SCSS we will loos mixins & placeholders for repeatative styles. Checkout placeholder & mixins and it's usage.


Complete Isolation

Another thing I suggest is allowing normalize: false in config to don't include normalization CSS. With vp-raw & this config we will be able to completely isolate component demos.

I am on the verge of releasing comp lib but I am stuck with this issue

@kiaking Please let me know your thoughts on this & PR

@kiaking
Copy link
Member

kiaking commented Jun 27, 2022

As said above we can automatically add :not psudo. But without SCSS we will loos mixins & placeholders for repeatative styles. Checkout placeholder & mixins and it's usage.

If we have nesting, why don't we just nest everything?

.vp-doc:not(.vp-raw) {
  h1,
  h2,
  h3,
  h4,
  h5,
  h6 {
    position: relative;
    font-weight: 600;
    outline: none;
  }
}

Another thing I suggest is allowing normalize: false in config to don't include normalization CSS. With vp-raw & this config we will be able to completely isolate component demos.

This is another issue indeed. Currently, when styles are imported from vitepress/theme directly, so I think it would be great to have a way to import styles separately, but we need think this in different way I guess (in different issue).

@jd-solanki
Copy link
Contributor Author

If we have nesting, why don't we just nest everything?

Providing vp-raw is not that easy. In my mentioned snippet we need to add class vp-raw along with vp-doc which will lose the purpose of vp-doc because when you apply it all styles will be gone from the start and it same as using a blank page. (IG vitepress has this kind of layout).

For this, we need a complex selector like Tailwind does: Playground

Hence, we need to append :not(:where([class~="not-prose"] *)) to each of selectors usedin vp-doc.

To resolve this we can post-process vp-doc selectors and append the above trick.

This is quite complex though. Let me know your thoughts on this.

@brc-dd
Copy link
Member

brc-dd commented Jun 27, 2022

Wouldn't just .vp-doc :not(.vp-raw) foo work? You'll have to create a container having vp-raw class like this:

<div class="vp-doc">
  ...
  <div class="vp-raw">
    <div class="foo">unstyled</div>
  </div>
  ...
  <div class="foo">styled</div>
  ...
</div>

Refer this for space in CSS selectors if you've some doubt: https://stackoverflow.com/a/58681511/11613622

Obviously this won't work if you directly add vp-raw to an element. You'll have to always wrap it inside some element on which you can put vp-raw class.

.vp-doc .vp-raw { all: unset; } can also be added to remove any styles from the container itself.

We don't have any styles on direct child of .vp-doc (always a div) so that also won't cause any issue either.

@jd-solanki
Copy link
Contributor Author

I am trying various solutions in this pen: https://codepen.io/jd-0001/pen/abqeLyK

Unfotunately, this is not simple as we thought. .vp-doc :not(.vp-raw) foo doesn't work as per above codepen.

all: unset isn't working either. It is removing user styles or default styles.

Right now I am busy with some other stuff but whenever I get time I will try to make another PR using PostCSS.

@brc-dd
Copy link
Member

brc-dd commented Jun 28, 2022

Ah right. Something like this should work though: https://play.tailwindcss.com/N2oYh1GHyt?file=css

Markup:

<div class="vp-doc">
  <div>
    <h1>Hey there</h1>
    <p>Hello world</p>
    <div class="vp-something">
      <h1>Hey there</h1>
      <p>Hello world</p>
    </div>
    <div class="vp-raw">
      <h1>Hey there</h1>
      <p>Hello world</p>
      <div class="user-something vp-raw">
        <h1>Hey there</h1>
        <p>Hello world</p>
      </div>
    </div>
  </div>
</div>

Styles:

h1,
p {
  color: tomato;
}

.user-something {
  font-size: 1.5em;
}

/* our styles below */
.vp-doc :not(.vp-raw) > h1,
.vp-doc :not(.vp-raw) > p {
  color: green;
}

Output:

image

The user will have to add vp-raw on all container elements.

@jd-solanki
Copy link
Contributor Author

If .vp-doc :not(.vp-raw) > h1 doesn't give any side effect than we can just append :not(.vp-raw) to .vp-row. We can also omit the postcss.

I will be tiny PR then 😉

Do you want me to make PR following your changes?

@brc-dd
Copy link
Member

brc-dd commented Jun 28, 2022

Do you want me to make PR following your changes?

If you have time then go ahead!

@brc-dd
Copy link
Member

brc-dd commented Jun 28, 2022

doesn't give any side effect

It will surely have specificity problems 😅. Adding :not increases specificity, the overrides we have in place may not work properly. It'll need investigation.

@jd-solanki
Copy link
Contributor Author

I will try my best to avoid side effects.

Thanks for the clean solution :)

@brc-dd
Copy link
Member

brc-dd commented Jun 28, 2022

Okay, so after some testing, I think the users can themselves add this short code to support vp-raw:

// docs/.postcssrc.cjs

module.exports = {
  plugins: {
    'postcss-prefix-selector': {
      prefix: ':not(:where(.vp-raw *))',
      includeFiles: [/vp-doc\.css/],
      transform(prefix, _selector) {
        const [selector, pseudo = ''] = _selector.split(/(:\S*)$/)
        return selector + prefix + pseudo
      }
    }
  }
}

Here is the complete demo: https://github.com/brc-dd/vp-raw

Adding this won't affect the specificity and all. We probably should add support for this in the router alone. Users can have the CSS part if they wish using the above config. This will prevent increasing the bundle size for those who don't need it.

@jd-solanki
Copy link
Contributor Author

Yes ❤️

It's working.

it's time to close this issue 🥳

What's next then?

We still have to deal with CSS normalization

@brc-dd
Copy link
Member

brc-dd commented Jun 28, 2022

I think CSS normalization can be left that way. All the selectors there are of lowest specificity, users will be able to override them without any issue.

@jd-solanki
Copy link
Contributor Author

I will wait for the update 😊

@brc-dd brc-dd removed the help wanted Extra attention is needed label Jul 8, 2022
@brc-dd
Copy link
Member

brc-dd commented Jul 28, 2022

Closing this for now. Will document it while resolving #591.

@brc-dd brc-dd closed this as completed Jul 28, 2022
@brc-dd brc-dd mentioned this issue Aug 2, 2022
2 tasks
@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
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants