[outline] Enable filtering the outline list #7088
Conversation
|
Can you let me know how this looks so far? I am still planning on writing some tests and maybe doing some small changes. |
| constructor(props: Props) { | ||
| super(props); | ||
| (this: any).filterOutlineItems = this.filterOutlineItems.bind(this); | ||
| (this: any).onUpdateFilter = this.onUpdateFilter.bind(this); |
kenjiO
Oct 2, 2018
Author
Contributor
If we use the ES6 arrow function definitions for filterOutlineItems and onUpdateFilter we wouldn't need to bind them here. Do you want to do this? If so do you want to change all the function definitions in the class to this style for consistancy?
If we use the ES6 arrow function definitions for filterOutlineItems and onUpdateFilter we wouldn't need to bind them here. Do you want to do this? If so do you want to change all the function definitions in the class to this style for consistancy?
| @@ -606,6 +606,10 @@ sources.header=Sources | |||
| # LOCALIZATION NOTE (outline.header): Outline left sidebar header | |||
| outline.header=Outline | |||
|
|
|||
| # LOCALIZATION NOTE (outline.placeholder): Placeholder text for the filter input | |||
| # input element | |||
flodolo
Oct 3, 2018
nit: you have two "input" in the comment
nit: you have two "input" in the comment
|
I love this @kenjiO ! Awesome work! |
828b70c
to
b01e92b
| if (e.key === "Escape" && this.props.filter !== "") { | ||
| e.preventDefault(); | ||
| this.props.updateFilter(""); | ||
| } |
kenjiO
Oct 9, 2018
Author
Contributor
I like having the user be able to press escape to clear the filter when it is in focus. There is an issue that escape already opens/closes the split console. I made it so if the user presses escape it clears the filter but doesn't toggle the split console. Then once the filter is cleared pressing escape furthermore toggles the split console. Is this OK to do?
I like having the user be able to press escape to clear the filter when it is in focus. There is an issue that escape already opens/closes the split console. I made it so if the user presses escape it clears the filter but doesn't toggle the split console. Then once the filter is cleared pressing escape furthermore toggles the split console. Is this OK to do?
AnshulMalik
Oct 10, 2018
Member
That sounds right to me
That sounds right to me
|
This is awesome :) |
4e466df
to
c606e8c
|
@darkwing I've finished getting this to filter the list by function names. At the call we discussed also filtering by class name but I don't know what the desired behavior for that would be. For example if the filter matched a class name would you show all the functions in that class or just show the class name? |
|
Wow, that's a great question. Let's focus on functions only for now, with the caveat that if the function was a class method, we should show the class name above it, IMO. Thoughts? |
|
@darkwing I agree at keeping it this way now. Usually there are not that many classes defined in a file so I don't think being able to filter them will be that useful. Currently if any class functions match the filter it will show all the class names with the matching functions underneath their associated class. I don't have any other pending changes for this PR so you can merge it if you don't need any further changes. |
Fixes #7054
Summary of Changes
Test Plan