feat(Pill): add new PillButton variant#1303
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
src/components/Pill/PillButtonEnhanced/__tests__/PillButtonEnhanced.spec.tsx
Fixed
Show fixed
Hide fixed
src/components/Pill/PillButtonEnhanced/__tests__/PillButtonEnhanced.spec.tsx
Fixed
Show fixed
Hide fixed
src/components/Pill/PillButtonEnhanced/__tests__/PillButtonEnhanced.spec.tsx
Fixed
Show fixed
Hide fixed
src/components/Pill/PillButtonEnhanced/__tests__/PillButtonEnhanced.spec.tsx
Fixed
Show fixed
Hide fixed
src/components/Pill/PillButtonEnhanced/__tests__/PillButtonEnhanced.spec.tsx
Fixed
Show fixed
Hide fixed
eszthoff
force-pushed
the
oneui-438-enhanced-pill-button
branch
from
615fb64 to
8ed2e96
Compare
| @@ -1,4 +1,3 @@ | |||
| /* eslint-disable @typescript-eslint/no-unnecessary-type-constraint */ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Priority is not aligned well:
There was a problem hiding this comment.
The clear Icon also doesn't seem to be perfectly align, but it is really minor thing. What is noticeable is that all the icons are too small, especially priority icon which is 12x12 ox and in design 20x20. The arrow one has onle 2 px difference
There was a problem hiding this comment.
the Clear one also should be 16x16
<Clear aria-label={clearLabel} viewBox="0 0 24 24" height="14px" width="14px" />
There was a problem hiding this comment.
The Prioirty button icon is coming from IconButton. I used the small, since the normal size is bigger then the pill button all together. Not sure where to go with it. Dima needs to align his designs :-D
I'll change the rest
There was a problem hiding this comment.
after making the icons 16px, I think the alignment is now ok, no?
There was a problem hiding this comment.
I think it should be some space between to button and text:
There was a problem hiding this comment.
yeah, with 16px, looks great. And agree with @mukiienko , some space would be nice 😊
There was a problem hiding this comment.
I will ask Dima to make a design proper with the priority icon, using the actual button icon. I didn't add the space, because... whatever, I'm not the designer, and I don't like to make up things :-D
| @@ -25,6 +24,7 @@ export interface Props<PriorityItemValue> extends DropdownMenuCheckboxItemProps | |||
| const { block } = bem('MultiSelectItem', styles); | |||
There was a problem hiding this comment.
It would be nice to have story with new pill and content dropdown to see how it works together.
There was a problem hiding this comment.
there is a story. you just have to choose the variant in the controls. Or is there something else?
There was a problem hiding this comment.
I meant when we click on the pill we show the dropdown:
There was a problem hiding this comment.
yes, I also mean that :D
There was a problem hiding this comment.
hmm, it's strange. Maybe something wrong on my machine. I have an error:
@AnastasiiaMP could you please check if it works for you?
There was a problem hiding this comment.
added the missing argument to the stories
There was a problem hiding this comment.
I still have the same error. Does it work for you in stories PillButtonEnhansed? @eszthoff
There was a problem hiding this comment.
Can you try now, I think you were looking before I pushed the changes.
There was a problem hiding this comment.
I don't have the errors now, but I still don't see the dropdown. Could you please check @AnastasiiaMP ?
There was a problem hiding this comment.
@mukiienko are you looking at this story? https://oneui-e9xydbbbf-textkernel-oneuis-projects.vercel.app/?path=/story/molecules-pill--pill&args=variant:enhanced
|
|
||
| export const MultiSelectItem = React.forwardRef( | ||
| /* eslint-disable-next-line @typescript-eslint/no-unnecessary-type-constraint */ | ||
| <PriorityItemValue extends unknown>( |
There was a problem hiding this comment.
The design looks different from the implementation; at least the cross icon has a different color:
There was a problem hiding this comment.
where do you have these colors? In Figma that is linked to the ticket they are grey.
https://www.figma.com/design/TlGOOkoCGSyDtrnMwmjnAl/OneUI-Components-v2?node-id=2649-6726&node-type=canvas&t=ZUTjsP5MwXspO6NW-0
There was a problem hiding this comment.
https://www.figma.com/design/4XoXmrgcJOSb1PEG8q75nB/OneUI-Components?node-id=2649-6726&node-type=canvas&t=cECzySOz2BGKjUqb-0
But maybe your design is latest, I don't know.
| @@ -25,6 +24,7 @@ export interface Props<PriorityItemValue> extends DropdownMenuCheckboxItemProps | |||
| const { block } = bem('MultiSelectItem', styles); | |||
There was a problem hiding this comment.
I meant when we click on the pill we show the dropdown:
3 participants
Story: ONEUI-438