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: use latest vuepress and remove unused features #254

Merged
merged 2 commits into from
Dec 17, 2024

Conversation

Mister-Hope
Copy link
Contributor

@Mister-Hope Mister-Hope commented Dec 15, 2024

see comments

@Mister-Hope Mister-Hope changed the title use latest vuepress feat: use latest vuepress and remove unused features Dec 15, 2024
@larshp larshp closed this Dec 16, 2024
@larshp
Copy link
Member

larshp commented Dec 16, 2024

redirects are removed

@mbtools
Copy link
Member

mbtools commented Dec 16, 2024

Hi @Mister-Hope

Marc here. I had the same initial reaction to your PR as Lars. It came out of nowhere and has no description at all. This is a very large diff too and I didn't see the redirects either. You have to scroll way down to find them.

Like Lars, I would have rejected this.

Your account name looks rather anonymous, too. So it's not apparent that you are from the Vue team. Now that I saw your profile the PR makes more sense. Some short description here would have definitely avoided this misunderstanding.

I will take a look at this update later in more detail.

@Mister-Hope
Copy link
Contributor Author

Mister-Hope commented Dec 16, 2024

I hate to make huge diffs, but obviously the project haven't maintain deps for quite a long time.

Also it might be bothering for both side to make tweaks PRs one by one especially they are relying on the former one.

Generally the changes are as follows:

  • Bump vuepress and my theme to latest vearsion
  • Move some plugin configuration from site config to theme config as they are supported by them already. redirect config is one of them
  • Tweaks options as they are a few breaking changes across versions like theme-color is moved to config style file from palette to generate a set of colors satisfying a11y.
  • remove unneeded options, like the de options, which has been dropped.
  • bump ci to node 22 as it does have better performance

I have some spare time last weekend, so I tried to help upgrading vuepress and my theme in some repos that have much stars.(see my timeline other PRs are made and some of them are already merged). I am sure I am doing everything well, and nothing is dropped without asking. Feel free to have a deep review if you think this is needed though.

@mbtools mbtools reopened this Dec 16, 2024
@mbtools
Copy link
Member

mbtools commented Dec 16, 2024

I tested it and technically it works great. Thanks for the migration!

There's a small issue with the red theme color. It used to render as #f03c2e. But when I run it on my machine, the result is a darker red. The "Getting Started" button is now #D61E0F and the font for links is now "#A7170C". It should all be the same red as the git logo.

You already have #f03c2e in the theme. I don't know why it's not working properly. Can you please help fix it?

Before:

image

After:

image

@Mister-Hope
Copy link
Contributor Author

Mister-Hope commented Dec 17, 2024

The color is meant to fix the background/text contrast, so we made a update for this, technically now a set of theme colors are automatically generated from the given theme color to ensure visual-inspired people can read the text clearly.

For the action button and the link text color, the original color does not fit this:

image

Users can still override them in palette file, I can update this now, but I need you to confirm this first. @mbtools

Since I met some visual-inspired and body-inspired friends while helping people, so VuePress ecosystem are focusing on a11y things.

@Mister-Hope
Copy link
Contributor Author

Mister-Hope commented Dec 17, 2024

Ref: https://dequeuniversity.com/rules/axe/4.9/color-contrast (from lighthouse)

@mbtools
Copy link
Member

mbtools commented Dec 17, 2024

That makes a lot of sense. I'm all for improving accessibility. Thanks again!

Copy link
Member

@mbtools mbtools left a comment

Choose a reason for hiding this comment

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

👍

@mbtools mbtools merged commit 3a540e1 into abapGit:main Dec 17, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants