Skip to content

Conversation

@codeMinter
Copy link
Contributor

No description provided.

@maxceem maxceem self-requested a review April 15, 2019 08:43
Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Works good @gurmeetb.

There is only one case when it's not completely works:

  1. If we enter a short username, then entering SPACE or ; doesn't work:
    image

    While autosuggestion works only after minimal text length AUTOCOMPLETE_TRIGGER_LENGTH. If we manually enter username it should work after we enter at least 1 character.

  2. Inside onInputChange method you return an empty string: return ''. As this method actually should not return anything, to avoid confusion, could you please don't return anything. You can use just return with nothing after it.

@codeMinter
Copy link
Contributor Author

codeMinter commented Apr 15, 2019

@maxceem fixed and also added support so users cannot enter first value as space or semi-colon, as discussed.

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Works great @gurmeetb.

Just fix two lint errors before I can merge it:

║ 20       │ 56       │ error    │ Unnecessarily quoted property 'label' found.           │ quote-props          ║
║ 20       │ 77       │ error    │ Unnecessarily quoted property 'value' found.           │ quote-props          ║

Thank you.

@codeMinter
Copy link
Contributor Author

@maxceem my bad, missed to run lint. Fixed it now.

Copy link
Collaborator

@maxceem maxceem left a comment

Choose a reason for hiding this comment

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

Thank you for quick fixes @gurmeetb.
Everything works good.

@maxceem maxceem merged commit 6b3a344 into topcoder-archive:cf16 Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants