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

feat(autocomplete-multiple): React component and tests #194

Closed

Conversation

juanigalan91
Copy link
Contributor

@juanigalan91 juanigalan91 commented Oct 16, 2019

Note: this PR heavily depends on PR #191 , in order to merge it, we need to finish review and merge that one first.

Created a multiple autocomplete component. It provides the following functionality:

  • Select multiple chips from a list of values
  • Allows new keyboard events such as backspace in order to remove the selected chips
  • Keeps all the functionality coming from the Simple Autocomplete component.

Use Cases:

Multiple Selection with keyboard and mouse navigation
znbEL8INfv

Creating new values
QQ5buyRZD1

Deleting selected values
FqFMH1hR4G

Keyboard navigation
lmUI1rkjhe

Side effects from the implementation:

  • Chip now has a isHighlighted prop that allows to set the css class into a focused state in order to allow deletion from the Autocomplete
  • TextField now receives a prop called before (similar to what Chip has) that allows to add a component before the input. With this, we can add the different selected chips before the input.
  • A refactor was done on the useKeyboardListNavigation hook in order to allow a simpler maintenance in the future, with regards to events tracked while using the hook

Note: there might be some css changes needed in order to complete the component:

  • Adjust spaces between chips
  • Adjust dropdown width in order to correspond to Zeplin specification
  • Make chips go down the text field and not to the sides when there are a lot of chips selected

@@ -29,7 +29,9 @@ interface IChipProps extends IGenericProps {
color?: Color;
/* Whether the chip has pointer on hover. */
isClickable?: boolean;
/** Indicates if the chip is currently in an active state or not. */
/** Indicates if the chip is currently in a highlighted state or not. */
isHighlighted?: boolean;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi ajouter ça ? Le simple fait de focus la chip ne suffit pas ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non, parce qu'on perd le focus sur le text field si on fait de focus en la chip, donc l'utilisateur ne peut pas continuer à écrire dans le texte field.

@juanigalan91 juanigalan91 force-pushed the feat/autocomplete-simple branch from 6368615 to b3a2924 Compare October 18, 2019 12:12
@juanigalan91 juanigalan91 requested a review from matmkian October 18, 2019 12:19
@juanigalan91
Copy link
Contributor Author

Closing this one since there were several changes after this implementation.

@matmkian matmkian deleted the feat/autocomplete-multiple branch December 6, 2019 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants