Skip to content

Conversation

@AlexandraBuzila
Copy link
Member

Remove filter from autocomplete renderer.

@coveralls
Copy link

coveralls commented Jan 14, 2019

Coverage Status

Coverage decreased (-0.1%) to 88.359% when pulling d266051 on AlexandraBuzila:fix-1182 into 28116d6 on eclipsesource:master.

- show vale list on select, but keep filtering active on keytype
@AlexandraBuzila
Copy link
Member Author

The updated commit keeps the filtering active while a key is pressed, but shows the full option list when the control is selected.

Copy link
Contributor

@edgarmueller edgarmueller left a comment

Choose a reason for hiding this comment

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

Works, but do you still have time left for adding a test case? If not, please open an issue for doing so.

export class AutocompleteControlRenderer extends JsonFormsControl {
@Input() options: string[];
filteredOptions: Observable<string[]>;
shouldNotFilter: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we remove the negation here and invert the logic here? It just keeps confusing me 😅

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed 😁
I also change the event type that is used for updating the filter from key up to key down, as the first key was otherwise ignored.

- update filter on key down events, as the first key is otherwise
ignored
@AlexandraBuzila
Copy link
Member Author

I created #1239 for adding tests for this.

@edgarmueller edgarmueller merged commit 1303781 into eclipsesource:master Jan 28, 2019
@edgarmueller edgarmueller added this to the 2.2.0 milestone Jan 30, 2019
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.

3 participants