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

improve filter input #59

Closed

Conversation

mushroomgenie
Copy link

Fixes #55

@mushroomgenie
Copy link
Author

@glaslos Was able to increase the width and add a reset query button on focus. Needed some clarity on the multiline input.

@glaslos glaslos requested a review from dkumiszhan July 3, 2023 10:07
@glaslos
Copy link
Member

glaslos commented Jul 3, 2023

Nice work! Regarding your question: Currently if you exceed the width of your input field, the text is pushed to the left out of the input field. I'd suggest wrapping the text and adding a new line.
We should also "shrink" the field once it loses focus to not impact visibility on the page.

@@ -6,6 +6,11 @@

let filter: string = '';
let filterValid: boolean = false;
let focus: boolean = false;

function handleFocus() {
Copy link
Member

Choose a reason for hiding this comment

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

For very simple handlers you can also define them inline

@dkumiszhan
Copy link
Collaborator

I am not sure if making input expandable now is useful. In the original version and Greynoise the query is within the header and by default does not occupy the whole line. There, it might be useful to expand the input. In the current version, there is nothing else on the same line next to the input. So I think we can consider 2 options:

  1. Move the filter input back to the header and do the expandable approach similar to what you are doing.
  2. Keep the filter input where it is but make it more expanded by default without the dynamic expanding.

Also it doesn't seem to apply filter when the input is expanded and "Reset Query" button doesn't remove all filters. I like how the reset button shows up on Greynoise. It would be nice if we could do something like that.

Could you also please fix the commits history, and rebase your code on top of what is already merged?

@glaslos
Copy link
Member

glaslos commented Jul 9, 2023

@mushroomgenie how about going for option 2 suggested by @dkumiszhan ?

@mushroomgenie
Copy link
Author

@glaslos sure we can do that.

@glaslos
Copy link
Member

glaslos commented Sep 1, 2023

Closing due to inactivity

@glaslos glaslos closed this Sep 1, 2023
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.

Improved filter input
3 participants