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

Global patterns for query and filter bar #1137

Merged
merged 24 commits into from
Sep 11, 2018
Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Aug 22, 2018

This PR is no longer a WIP, but it does still have some issues that need to be worked out.

Dependent on #1139 going in first
Dependent on #1173 going in first

The main focus of the work is on the filter bar. The query bar's auto-complete can be re-used from current implementation.

screen shot 2018-08-23 at 18 55 22 pm

The components for this pattern are contained in the docs folder, but if need be, can be moved to EUI components.


I still need to:

  • Check IE (and other non-chrome browsers)

All that being said, this is VERY close to finished, so cc'ing @Bargs as well for initial feedback of feasibility in using this as a pattern to replace the current implementation.

@cchaos cchaos requested review from snide and ryankeairns August 22, 2018 21:14
Copy link
Contributor

@ryankeairns ryankeairns left a comment

Choose a reason for hiding this comment

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

Some quick reactions:

  • do all the tags start with "@tags." and if so, is that detail necessary?
  • I prefer the strikethrough over the circle with dash icon. Another option that might be even more clear would be text like "keyword is value" , "keyword not value". Then you could potentially do without the strikethrough and the icon.
  • The full-height divider bar (and removal of the padding on pinned side of the bar) was still clearer in my personal opinion
  • One last highly subjective piece of feedback :) , I want to like the neon aqua blue, but it feels a bit rich. Perhaps a more cool tone would do the trick.

It's looking great!

@cchaos
Copy link
Contributor Author

cchaos commented Aug 22, 2018

Thanks @ryankeairns

  • Not they don't all start with @tags, in fact I don't think any actually would. To see what we're re-designing you can go to this dashboard and start playing with the "Add a filter" button at the top.
  • The current one actually does change the wording but they do it like: key: value to NOT key:value... not sure what direction is best
  • Yeah I couldn't actually do the dividing border since all the filters are inline and the bar can grow in height.

@ryankeairns
Copy link
Contributor

oh jeez, I now totally see that it was sample text :D

@ryankeairns
Copy link
Contributor

Ok, took a peak at the current design.
The current text using key: value and NOT key:value was easier to read than the icons. Again, I'm the equivalent of a first time user here, but the icons (and strikethrough) are adding more noise to an already busy bar of text. I think in this case the text alone does the trick.

@cchaos
Copy link
Contributor Author

cchaos commented Aug 22, 2018

Yeah, you're probably right. How about the disabled (invisible) ones?

@ryankeairns
Copy link
Contributor

In this case, the icon and gray colors feel appropriate and effective.

Given the limitations of gray text and retaining accessible contrast, the icon feels like a good alternative. Additionally, limiting icon usage to this single (presumably less frequent) use case makes it stand out more without the aforementioned downside of creating too much noise.

@snide
Copy link
Contributor

snide commented Aug 22, 2018

This is looking sweet. Agree on the icons / strikethrough feedback from @ryankeairns. I tried to take a stab at the pinning stuff. It's pretty difficult to solve. Here are some slants I went through.

Prolly is the most clear

Eh? Eh...? Ehhhh I know it's super spacious, but it should be clear and scale ok. I know this is gonna sound silly, but it might also give us some range if we wanted to do AND/OR style grouping in a UI format.

Oh, and I'd only show the actions and filter add on the row you were hovering on. Has the side benefit of letting you add stuff to the pinned set immediately.

image

Prolly has trouble scaling with lots of items

image

@cchaos
Copy link
Contributor Author

cchaos commented Aug 23, 2018

Hmmm, I'd hate to give up that much vertical space for what could be just one filter. I also think the second options would end up basically like the first when the second box gets full. If we can, I'd like to try to keep them all in one area/maintain the pinned signifier as part of the filter itself.

I got some inspiration and played around with styling the badges differently from the default badge we have. Here are some ideas.

No form background

This one gets rid of the form input wrapping container and adds a little more shadow to the actual filters.
image

Slight hue colors

This one keeps that container but gets rid of the borders around the filters and inverts the pinned version.
image

@cchaos
Copy link
Contributor Author

cchaos commented Aug 23, 2018

I also think we should keep the (x) remove button always on the filter otherwise it's really hard to quickly remove a bunch (without removing all at once).

@snide
Copy link
Contributor

snide commented Aug 23, 2018

I like the one with no wrapping input a lot. 👍

@ryankeairns
Copy link
Contributor

Oooo, really liking these options!

@snide
Copy link
Contributor

snide commented Aug 23, 2018

Somewhat related, had to put something together for logging that allows for highlighting separate of the query bar (think "hosts: all" and then highlight the term "error"). Right now I have it as something that appears after the KQL bar since it's an operation that happens after the filtering. I don't think they'll ever have the secondary filter bar so i don't think the two will ever be seen at once. Space is at a premium in logging, which is why it's all done in a popover.

https://codesandbox.io/s/617mv4p62w

@cchaos cchaos mentioned this pull request Aug 23, 2018
@cchaos cchaos requested a review from chandlerprall August 23, 2018 23:04
@cchaos cchaos changed the title [WIP] Global patterns fo query and filter bar Global patterns fo query and filter bar Aug 23, 2018
@cchaos cchaos changed the title Global patterns fo query and filter bar Global patterns for query and filter bar Aug 23, 2018
@cchaos
Copy link
Contributor Author

cchaos commented Aug 23, 2018

No longer POC, but kinda WIP

instead of border because a border was causing the box-shadow to be off
PropTypes.shape({
asPlainText: PropTypes.bool,
}),
PropTypes.object,
Copy link
Contributor

Choose a reason for hiding this comment

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

this looks like a bad merge with master? let's keep the shape

@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2018

Ok, this is ready for final (and code) review.

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

LGTM (code review, pulled locally), added one small comment;
this looks beautiful

}

componentDidMount() {
// Simulate initial load.
Copy link
Contributor

Choose a reason for hiding this comment

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

remove if not needed

@cchaos
Copy link
Contributor Author

cchaos commented Sep 11, 2018

Thanks all! I'm going to merge this one in. It may develop further as @Bargs starts using it.

@cchaos cchaos merged commit 18575e7 into elastic:master Sep 11, 2018
@cchaos cchaos deleted the global-patterns branch September 11, 2018 20:55
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.

4 participants