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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: upgrade vitepress and add aside sponsors #8363

Merged
merged 4 commits into from
May 28, 2022

Conversation

kiaking
Copy link
Contributor

@kiaking kiaking commented May 28, 2022

Description

Upgrade VitePress and add sponsors at doc page! Many things have been updated.

  • Sponsors below doc outline.
  • Carbon Ads added.
  • Sidebar font side is a bit bigger to increase readability.
  • Refined nav bar menu just a bit (font size and appearance).
  • Code syntax highlight is now using Shiki instead of Prism.
  • Nicer 404 page design.
  • "type": "module" project support

Screen Shot 2022-05-28 at 17 45 46

Question

Now VitePress can run on project with "type": "module". Do you want to set it to root package.json? I don't know if there's any meaning to it but... 馃

Only benefit from my side is that we have to set "type": "module" to the root package.json when we want to "locally link VitePress" and use VitePress's dev mode while developing the doc, because VitePress doesn't build cjs modules while on dev mode. This is required only when we want to modify VitePress code and Vite docs at the same time.

However, I think I'm the only one who would be doing that so... 馃槄


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@patak-dev
Copy link
Member

Looks awesome!

I think @antfu changes in Vite will end up with us setting "type": "module" in the root. We should do it later in a separate PR though.

@kiaking
Copy link
Contributor Author

kiaking commented May 28, 2022

Thanks! So I think There's no update needed 馃憤 (I have no idea why test is failing but...

@patak-dev
Copy link
Member

@kiaking the lint error is there because of a formatting issue. You need to run pnpm run format. Looks like the commit hooks aren't working correctly, dominikg mentioned that we missed one ts-node there (we are now using esno). Sorry about that.
The Mac issue is across the board, don't worry about it. The Windows issue seems legit though. Did you add some new path processing that may require normalization?

@patak-dev
Copy link
Member

About the aside sponsors, maybe they could stick to the bottom as much as possible so we avoid having layout jumps when changing pages?

@kiaking
Copy link
Contributor Author

kiaking commented May 28, 2022

Thanks! Run format and pushed. Sorry about that 馃槼

Did you add some new path processing that may require normalization?

No idea! But that's fine I usually run format by hand so 馃憤

About the aside sponsors, maybe they could stick to the bottom as much as possible so we avoid having layout jumps when changing pages?

Nice idea. Let me try. I think it's possible by using flex-grow on outline component... 馃

Currently working on with some other stuff so maybe not soon but soon enough! If this gets merged, I'll include that in another patch 馃憤

@kiaking
Copy link
Contributor Author

kiaking commented May 28, 2022

Oh, seems like doc fails to build on windows... 馃 Hmmm... anyone know how to fix this? (do we need doc build test on windows 馃槼 ?

@patak-dev
Copy link
Member

You can run npx simple-git-hooks to update the git hooks, the hooks are fine, but every time they change a manual step is required (the idea is that they shouldn't change often)

The normalization of paths was about the docs error here:

build error:
 Error [ERR_UNSUPPORTED_ESM_URL_SCHEME]: Only URLs with a scheme in: file, data are supported by the default ESM loader. On Windows, absolute paths must be valid file:// URLs. Received protocol 'd:'

We started to run build docs in Vite's CI. Looks like there is an issue with Windows in this VitePress version

@kiaking
Copy link
Contributor Author

kiaking commented May 28, 2022

Yeah seems like. I just got issue on VitePress side 馃槄

@userquin
Copy link
Contributor

userquin commented May 28, 2022

Oh, seems like doc fails to build on windows... 馃 Hmmm... anyone know how to fix this? (do we need doc build test on windows 馃槼 ?

we use patchWindowsImportPath on vitest, I can check your branch on Windows:

function slash(str: string) {
  return str.replace(/\\/g, '/')
}
function patchWindowsImportPath(path: string) {
  if (path.match(/^\w:\\/))
    return `file:///${slash(path)}`
  else if (path.match(/^\w:\//))
    return `file:///${path}`
  else
    return path
}

@kiaking
Copy link
Contributor Author

kiaking commented May 28, 2022

Upgraded VitePress and Windows test is passing, but seems like to failing on playground/css test 馃槗

However, I also added hero image to the home page 馃槼

Making sponsors on aside stick to the bottom is not included yet. We have nice fader gradient on the bottom of aside so users can see it's scrollable, and trying to find a way for this to not interact with sponsors and ads 馃

Screen Shot 2022-05-29 at 3 57 52

@patak-dev
Copy link
Member

I'm running out of adjectives! I love the new landing page 馃槏
Ignore the CSS test, we need to check why it has been flaky lately

@patak-dev patak-dev merged commit 8f18fe7 into vitejs:main May 28, 2022
@kiaking kiaking deleted the docs-vitepress-update branch May 28, 2022 19:46
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.

None yet

3 participants