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

docs: Expanded sections on applying custom CSS and SCSS to the default theme, and working with dark mode #2377

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

SimonEast
Copy link

Hi VitePress team,

I spent some time figuring out how to apply style customizations to the default theme, and would like to contribute back to the official documentation to assist others in this process.

Here's my pull request, which relates only to the docs.

Since this is my first one, please feel free to provide feedback or tweaks/adjustments to what I have written, and I can make any necessary changes if required.

Cheers.


Review PR in StackBlitz Codeflow Submitted with StackBlitz Codeflow.

@SimonEast SimonEast changed the title Docs improvements: how to apply custom CSS and SCSS to the default theme, including working with dark mode docs: expanded sections on applying custom CSS and SCSS to the default theme, and working with dark mode May 16, 2023
@SimonEast SimonEast changed the title docs: expanded sections on applying custom CSS and SCSS to the default theme, and working with dark mode docs: Expanded sections on applying custom CSS and SCSS to the default theme, and working with dark mode May 16, 2023
Minor correction, adding extra CSS variable, and correcting typo
@brc-dd brc-dd force-pushed the main branch 2 times, most recently from 7ad4a4e to f0d3082 Compare August 3, 2023 10:20
@github-actions github-actions bot added the stale label Nov 6, 2023
@github-actions github-actions bot removed the stale label Nov 25, 2023
@github-actions github-actions bot added the stale label Feb 2, 2024
@github-actions github-actions bot removed the stale label Apr 28, 2024
@SimonEast
Copy link
Author

I've updated my PR so that the docs match the latest VitePress theme. Would be great to get any final feedback/tweaks that might be preventing it from being merged in.

I think it's important to get people up and running fast with the default theme but allow them to easily swap the color scheme for something that matches their project, and that's what this PR is intended to do.

docs/reference/default-theme-config.md Outdated Show resolved Hide resolved
@@ -17,7 +17,7 @@ export default {
}
```

**The options documented on this page only apply to the default theme.** Different themes expect different theme config. When using a custom theme, the theme config object will be passed to the theme so the theme can define conditional behavior based on it.
**The options documented on this page only apply to the default theme.** Different themes expect different theme configuration options. When using a custom theme, the theme config object will be passed to the theme so the theme can define conditional behavior based on it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the earlier one was already clear IMO

Copy link
Author

@SimonEast SimonEast Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well the first few times I read that sentence, it stumped me. I thought What does that even mean? I thought "configuration options" would be a small improvement and more accurate term.

But I can skip the change if necessary.

@@ -1,6 +1,6 @@
# Default Theme Config

Theme config lets you customize your theme. You can define theme config via the `themeConfig` option in the config file:
Theme config lets you customize your theme. You can define theme config via the `themeConfig` option inside [`.vitepress/config.[extension]`](./site-config):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't think this is needed either

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps it's obvious once you've used VitePress for a while, but as a newcomer it's not clear which config file this is referring to and where it's located. I initially wondered "Does VitePress have more than one config file?" and "Is there a theme config file, or is it the same as the VitePress config file?" and "Which folder should I be looking in?".

I think having the path here reduces that initial confusion.

docs/guide/using-vue.md Outdated Show resolved Hide resolved
@@ -200,9 +200,9 @@ Note that this might prevent certain tokens from being syntax highlighted proper

## Using CSS Pre-processors

VitePress has [built-in support](https://vitejs.dev/guide/features.html#css-pre-processors) for CSS pre-processors: `.scss`, `.sass`, `.less`, `.styl` and `.stylus` files. There is no need to install Vite-specific plugins for them, but the corresponding pre-processor itself must be installed:
VitePress has [built-in support](https://vitejs.dev/guide/features.html#css-pre-processors) for CSS pre-processors: `.scss`, `.sass`, `.less`, `.styl` and `.stylus` files. Simply install the pre-processor itself using the applicable command:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

again unnecessary change. please avoid just rephrasing and moving things around

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt the "Vite-specific plugin" point was a side-note that just added confusion when I initially read it (it led me to ask "Am I supposed to understand the difference between a Vite-plugin and a pre-processor?"). The revised version jumps straight to the commands necessary to get them up and running, and puts the side-note below, for those needing it.

But if you feel strongly about keeping it as is, I can revert.

@@ -4,7 +4,7 @@ outline: deep

# Extending the Default Theme

VitePress' default theme is optimized for documentation, and can be customized. Consult the [Default Theme Config Overview](../reference/default-theme-config) for a comprehensive list of options.
VitePress' default theme is optimized for documentation, and a number of elements can be customized via `.vitepress/config.[extension]`. Consult the [Default Theme Config Overview](../reference/default-theme-config) for a comprehensive list of options.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

site config has a separate section, no need to keep putting same thing everywhere.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As mentioned in another comment, I initially found it confusing to work out which config file I was supposed to modify. Just trying to help new users get up to speed quickly.

If you're opposed to the repetition, perhaps it could just be a hyperlink instead, like this:

Suggested change
VitePress' default theme is optimized for documentation, and a number of elements can be customized via `.vitepress/config.[extension]`. Consult the [Default Theme Config Overview](../reference/default-theme-config) for a comprehensive list of options.
VitePress' default theme is optimized for documentation, and a number of elements can be customized via [the site config file](../reference/site-config). Consult the [Default Theme Config Overview](../reference/default-theme-config) for a comprehensive list of options.

docs/guide/extending-default-theme.md Outdated Show resolved Hide resolved
Comment on lines +25 to +26
::: code-group
```js [.vitepress/theme/index.js]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code groups are not meant to just show title, keep the original formatting

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, I find it cleaner and clearer to have the filename outside the actual code block. It also means that when you click the "copy" button, you don't have the additional comment included. I think using a code-group tab for this purpose is a clean solution, but if you're against that I can revert to the original.

Comment on lines +126 to +128
::: tip
When SASS variables or functions are used to define a CSS variable, they need to be wrapped in `#{ ... }`. See the SASS documentation [here](https://sass-lang.com/documentation/breaking-changes/css-vars) and [here](https://sass-lang.com/documentation/variables).
:::
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove this, we don't need to teach sass/scss to users. if someone is using a pre processor they should be aware of it's syntax

Copy link
Author

@SimonEast SimonEast Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree that it's not necessary to explain basic SCSS syntax for the most part, however in this case I believe explaining the #{ ... } syntax is warranted since:

  • It's difficult to find within the SASS documention. It's hidden under a "breaking changes" document, and not mentioned at all on the main SASS Variables doc page where you would expect.
  • Combining CSS variables and SASS variables may not be standard practice for many devs (generally you would stick to one or the other), but when customizing VitePress in this way it becomes necessary

Having this tip here would have saved me 20mins of hunting through my CSS inspector for why it wasn't working, and then paging through multiple Google results until I found the issue.

@brc-dd brc-dd added the needs author action The PR is not ready yet label Jun 7, 2024
@SimonEast SimonEast requested a review from brc-dd June 7, 2024 10:58
@brc-dd brc-dd removed the needs author action The PR is not ready yet label Jun 9, 2024
@github-actions github-actions bot added the stale label Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants