-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
feat: Copy code button #8995
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: Copy code button #8995
Conversation
|
I don't know why, but it seems the file logic isn't working for some snippets: https://svelte-gz1whcs22-svelte.vercel.app/docs/svelte-components#script . Could you investigate? |
Happening cuz of cache on vercel. The script adds a class EDIT: Rebuilt without cache, check again https://svelte-dev-git-feat-copy-code-button-svelte.vercel.app/docs/svelte-components |
Good to go from my side! |
The file names were my idea, as I felt that having copy buttons everywhere was adding visual clutter, and given we already have these in SvelteKit, we might aswell add it to th Svelte core docs and then add the copy button on the right. More opaque copy code button would be good either way I think, I'd say visual clarity should outweigh accessibility concerns here, especially since this is not a critical part of the app. |
Because then it would be applied to one-liners or fractions of code snippets, too, where it doesn't make sense and just creates visual clutter. It was that way initially and it looked bad, see this comment: #8995 (comment) |
I believe Dominik meant why it is: |
full line fences add "visual clutter" already, the copy button in itself doesn't change that much, esp if it is the less dominant variation Rich suggested. And who are we to judge what the user wants to copy or not. having these copy buttons only in some places adds inconsistencies too, which isn't great for UX either. |
The snippets in that picture are no real code snippets, they are part of the API definition. So they shouldn't look like code snippets which a copy button makes them look like more. |
looking at https://svelte.dev/docs/svelte-transition i can understand why you are afraid of adding more visual noise. There are no less than 4 different visual appearances of code
Important links like the related tutorial section and the transition directive these are based off are not standing out against simple type definition links. So i agree adding copy buttons on every block of that page would make the situation even worse. But the solution isn't littering the rest of the docs with opt-in syntax to show a copy button. You can either add an opt-out there - or - which would be my preferred approach, redo these sections to be more approachable, have less code blocks and then having a copy button on them suddenly isn't noise anymore but the helpful feature we want it to be... |
We can set copy: true by default for every snippet and add |
Hey folks, what's the verdict on this? Do I move forward with reverting |
sounds like an initial plan to me
sounds like a plan, i'd add a slight update on the copy button style so it's a bit more mute/neutra how much work would the pragma thing be though? slightly related for code block rendering: https://github.com/antfu/shikiji |
WDYM? Changing it from
Ohh this looks good. Although shiki-twoslash uses shiki internally. I wonder if there's a way to swap shiki with shikiji in that department 🤔 |
Applied the discussed changes.
|
* Push * Bump site-kit * Add headers to primary snippets * Update deps * Bump deos * redploy * Back to normal * Push * Bump deps * site: fix rendering of promise in deprecation warning (sveltejs#9191) * copy: true * Bump site-kit * Use cache
Related sveltejs/site-kit#175
Before submitting the PR, please make sure you do the following
feat:
,fix:
,chore:
, ordocs:
.Tests and linting
pnpm test
and lint the project withpnpm lint