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(playground): fix github button style #7722

Merged
merged 3 commits into from
Jul 9, 2023
Merged

fix(playground): fix github button style #7722

merged 3 commits into from
Jul 9, 2023

Conversation

BeADre
Copy link
Contributor

@BeADre BeADre commented Feb 15, 2023

WechatIMG7893

The size of button content is larger than inner a tag and has set cursor: pointer so when we click the padding part of button will not open github

>
<GitHub />
</a>
<button title="View on GitHub" class="github" @click="openGithub">
Copy link
Member

Choose a reason for hiding this comment

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

Using button instead of a tag to open a new page is not conducive to SEO, maybe you need to rethink how to solve it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The source code href on github really need to SEO? 😲😲

@Justineo
Copy link
Member

We should just remove the <button> outside the <a> and update the styles.

@BeADre BeADre changed the title fix(playground): add function to open github fix(playground): fix github button style Feb 16, 2023
@BeADre
Copy link
Contributor Author

BeADre commented Feb 16, 2023

@Justineo done


.links .github:hover {
color: var(--highlight);
}
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to merge this into L292:300:

.links > button,
.links > a {
  padding: 1px 6px;
  color: var(--btn);
}

.links > button:hover,
.links > a:hover {
  color: var(--highlight);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@sxzz sxzz added the ready to merge The PR is ready to be merged. label Jun 12, 2023
@yyx990803 yyx990803 merged commit 5ee992c into vuejs:main Jul 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready to merge The PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants