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(theme): "copy code" button not readable on hover state (#819) #1998

Merged
merged 3 commits into from
Feb 28, 2023

Conversation

kiaking
Copy link
Member

@kiaking kiaking commented Feb 27, 2023

fix #819
fix #1892

This PR does 2 things.

  1. Increase the code block height a bit and move "lang" label up to avoid lang label over wrapping with the code.
  2. Adjust copy code button style so that it's more readable on hover style (copy to clipboard button style #819).

Screenshot 2023-02-27 at 19 20 32

Screenshot 2023-02-27 at 19 20 39

Screenshot 2023-02-27 at 19 20 53

Screenshot 2023-02-27 at 19 21 05

@kiaking kiaking added bug theme Related to the theme labels Feb 27, 2023
@kiaking kiaking requested a review from brc-dd February 27, 2023 10:27
@kiaking kiaking self-assigned this Feb 27, 2023
@brc-dd
Copy link
Member

brc-dd commented Feb 27, 2023

Rest LGTM. Closes #1892 too IG?

@kiaking
Copy link
Member Author

kiaking commented Feb 27, 2023

Thanks! Ahhh #1892. I'll double check this to see if it works with this change 🤔

kiaking and others added 2 commits February 27, 2023 19:37
Co-authored-by: Divyansh Singh <40380293+brc-dd@users.noreply.github.com>
Co-authored-by: Divyansh Singh <40380293+brc-dd@users.noreply.github.com>
@brc-dd
Copy link
Member

brc-dd commented Feb 27, 2023

@kiaking
Copy link
Member Author

kiaking commented Feb 27, 2023

OK I've tweaked a bit and checked. Now the current one is not that bad but not perfect I would say. If we increase the top padding to 24px and then move the lang label up for another 2px looks perfect, when there is line hightlight on the first line.

But then the rest of code block with no line highlight looks a bit worse (in my opinion) 🤔 Top/Bottom padding would be too big, and lang label is too near the edge.

So, I say, having line highlight on the very first line should be rare, therefore the current stye is best for overall design 😳

@kiaking kiaking merged commit c2de4ca into main Feb 28, 2023
@kiaking kiaking deleted the 819-copy-code-not-readable-on-hover branch February 28, 2023 00:46
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme Related to the theme
Projects
None yet
Development

Successfully merging this pull request may close these issues.

copy to clipboard button style
2 participants