Add ChipInput component #808
Conversation
princerajpoot20
requested review from
fmvilas,
boyney123,
mcturco,
magicmatatjahu,
KhudaDad414 and
asyncapi-bot-eve
as code owners
|
✅ Deploy Preview for asyncapi-studio-design-system ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for modest-rosalind-098b67 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
packages/ui/components/ChipInput.tsx
Outdated
| initialChips?: string[]; | ||
| initialInputValue?: string; |
There was a problem hiding this comment.
I suggest we add support for the following props:
| initialChips?: string[]; | |
| initialInputValue?: string; | |
| name: string; | |
| id: string; | |
| className: string; | |
| onChange: function; | |
| isDisabled: boolean; | |
| initialChips?: string[]; | |
| initialInputValue?: string; |
packages/ui/components/ChipInput.tsx
Outdated
| const [chips, setChips] = useState<string[]>(initialChips); | ||
| const [inputValue, setInputValue] = useState<string>(initialInputValue); |
There was a problem hiding this comment.
the state of this component is going to be managed from outside. no state management should be done in the component.
packages/ui/components/ChipInput.tsx
Outdated
|
|
||
| return ( | ||
| <div className="flex flex-wrap items-center p-2 bg-gray-900 rounded"> | ||
| <div className="w-full text-gray-100 mb-2">Tags</div> |
There was a problem hiding this comment.
| <div className="w-full text-gray-100 mb-2">Tags</div> |
packages/ui/components/ChipInput.tsx
Outdated
| }; | ||
|
|
||
| return ( | ||
| <div className="flex flex-wrap items-center p-2 bg-gray-900 rounded"> |
There was a problem hiding this comment.
based on the design, this component should have a border as well.
|
@KhudaDad414 Thankyou for the review. I have made the changes 🙂 |
There was a problem hiding this comment.
Left some comments.
I also noticed that if I delete the text and press delete again, it doesn't remove the last chip. I was expecting to be able to remove chips with the keyboard.
Also, I should be able to navigate through the chips using the keyboard (tab key) and remove a chip if needed (probably the delete key).
Hope that makes sense. It's looking awesome so far
packages/ui/components/ChipInput.tsx
Outdated
| className="p-1 bg-gray-900 text-white rounded outline-none" | ||
| placeholder={placeholder} | ||
| disabled={isDisabled} | ||
| defaultValue={'registr'} |
There was a problem hiding this comment.
I would not default it to this but an empty string. You can then create multiple cases in the design system:
- How it looks with all the defaults
- How it looks with some default text
- How it looks with a custom placeholder
- And so on...
|
@fmvilas Thank you for the review. 🙂
Definitely makes sense. My bad. I missed these things. About navigating through chips, it's a great point that I missed out on. I use tab a lot to navigate. it just makes it so convenient. |
princerajpoot20
force-pushed
the
ChipInput
branch
from
15cbda8
to
ac1e8b7
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
|
/rtm |
Related issue(s)
Resolves #753