chore: merge shellbox into codebox #5425
Conversation
|
@araujogui is attempting to deploy a commit to the OpenJS Foundation Team on Vercel. A member of the Team first needs to authorize it. |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Can on dark theme the button to have this background and colour?
Also, the "copy" text should be "copy" not "Copy" or we can use a Copy emoji/icon from React Icons (I rather the emoji/icon!) |
|
Also what about the selected option also have a darker colour? (Screenshot below with the change)
The colour is |
|
Also if we use an icon for the copy/paste button instead of text (which I think is better) we need to remove the translation. (And also change the width of the button) |
|
Personally -1 for the icons on button. Copy is more explicit that an icons. Then there's nothing specific about the design system. |
A copy icon is a very standard practice. Is actually the most common thing done out there. (Example from Next.js docs)
|
|
Like something like this But maybe floating or hmmm |
|
With more padding and small increase of size and ok |
|
My screenshot was just an example. Ideally the way of the Next.js docs would be great. But we don't have a designer, so... |
|
I'm learning design with Figma. Are you interested by I mock-up ? |
Thanks, but we don't need that for now. The Foundation is also in talks on providing a Designer, so that's awesome news. |
+1 for using an copy emoji |
|
cc @araujogui any updates here? |
|
Also, @araujogui can we create a prop that allows us to disable the "header" of the Codebox (like
Also when the header is hidden the copy button should only appear when hovering and should be inline with the code like in the screenshot below,
|
|
Also on Light Mode, the copy button background feels weird. It doesn't fit. It should be a lighter green variation.
|
chore: delete shellbox component chore linting test: create textToCopy tests feat: wip feat: add copied icon feat: fix colors fix: increase copied timeout feat: add aria label refactor: remove shellbox i18n refactor: linting fix: some colors test: fix copy find chore: update snapshot fix: lighter copy button feat: add hideheader prop feat: trim and remove $ chore: fix linting chore: update snapshots fix: button display
0c87bfb to
dc66e40
Compare
Yeah I've seen a technique where it's visually present but not copy-able. And in case people don't know (because I didn't forever), the prompt does serve some purpose, if however small, in showing that |
|
Well this PR fixed the $ issue but only for $. Maybe what we need in the future is a field called prefix that allows us to add non-cooyable pieces |
|
I’m still not fan of Lang button. I propose to glue them and use an separator like | |
Please open the latest version of Storybook before commenting this. |
|
In lasted version of storybook it’s isn’t glue |
|
@araujogui a tiny bug the third one should not have left border-radius and the second one should.
|
Yes, they are. Or what do you mean with "glue" if this is not what you're referring to? |
|
idk but it’s render like that on my phone |
|
It is working as expected on my "mobile" viewport. @araujogui are you able to reproduce this? @AugustinMauroy can you tell me which browser you're using? |
|
Safari lasted on iPhone 7 (I’m on lasted storybook deploy) |
I can't reproduce this on Safari macOS, nor on Safari on my iPhone 14 Pro (iOS 17) |
|
I'm merging this as it is now, but @araujogui, can you make a follow-up PR with the tiny bug fix I've mentioned? This PR just had been sitting for so long, and we can reiterate the minor bugs on follow-up PRs. |
Description
Merge Shellbox into Codebox
Related Issues
Check List
npx turbo lintto ensure the code follows the style guide. And runnpx turbo lint:fixto fix the style errors if necessary.npx turbo formatto ensure the code follows the style guide.npx turbo testto check if all tests are passing, and/ornpx turbo test:snapshotto update snapshots if I created and/or updated React Components.