Adds manual, editable, auto-suggested filters, and negated&globbed path-based filters

[Vigasaurus]

Changes

Remade version of #1089 as the revert + the conflicts + something weird while resolving broke it

Notes:

• I still need to go back and verify that everything merged in correctly, but it should be okay
• I currently have package-lock.json deleted, because npm i was throwing me a peer dependency error between tailwind 2.1.2 and aspect-ratio 0.2.1, but I think that's present in master too (or it could just be my version of npm, I dunno)

Tests

• Automated tests have been added

Changelog

• Entry has been added to changelog

Documentation

• Docs have been updated

[ukutaht]

Looks good to me overall, it's getting very close. I left a couple small style comments in the code. Here are my comment about the overall behaviour:

I like how the select box works now. However, I am very much annoyed by the slight differences in border radius, border color, font size, padding, chevron color etc between these two inputs. They should be consistent.

The loading indicator is nice 👌. A small thing I noticed is when you select a value from the dropdown, the loading indicator will flash for a second. I can also see a call to the suggestions endpoint. This seems unnecessary after the user has already made a choice.

Personally I don't like the focus style on the dropdown (in light mode, at least). I think it's bg-indigo-300 with dark text. I've created a JS dropdown for the invitations UI and I used bg-indigo-600 text-white as the hover/focus style which I think fits much better. Thoughts?

[ukutaht]

Can this be done in a single setState? The current flow seems to be:

setState({selectedFilter}) -> componentDidUpdate() -> setState({updateValue, inputValue, negated, forceReloadSuggestions})

Why can't it be just:

setState({selectedFilter, updateValue, inputValue, negated, forceReloadSuggestions})

when the filter input changes?

[Vigasaurus]

I mostly wrote it like this to make it a bit more DRY - because the selectedFilter could change both via the dropdown onChange, or from componentDidMount when loading from the URL param - it could theoretically be brought out, but would end up being duplicated.

[ukutaht]

What is this variable doing?

[Vigasaurus]

This is just to force react-select to re-fetch the suggestions when the input changed externally (when loading from the currently set filter, primarily), using the key to force it to do so. They don't expose a way to do this directly, but it's well documented that changing the key of the component forces it to re-fetch them.

[ukutaht]

What's the reason for adding this new definition for page_times_by_page_url?

[Vigasaurus]

This is for the negated and/or globbed versions of the page filter - because the old version relied on a static list of page paths from top_pages to pick which times to keep - this version is for the top stat, which passes in nil as the page_list, then the filter is used to decide which pages to keep, then they're averaged into the time on page top stat for a negated or globbed page path (since this type of filter can't be represented in a static page_list, whereas from top_pages` it's always just the up to 100 pages being returned at that call).

[ukutaht]

I did a trial with downshift instead of react-select and it seems pretty neat. Small win in bundle size but the big win I think is that it's headless so we have full control over the DOM and we can use Tailwind directly for rendering instead of !important classes.

[ukutaht]

What's the purpose of tracking inputValue and updatedValue in the state? Isn't react-select already tracking the input value? Can this be removed from the state?

[Vigasaurus]

The disparity between updatedValue and inputValue is a remnant of an old implementation that I've now combined into just updatedValue, but we keep track of that (and pass it into react-select as a prop) instead of letting react-select do so, because we need to update the input manually at times (to help it not be blank when re-editing, when using the filter edit flow), otherwise it'd always default to blank when going into the input.

[Vigasaurus]

However, I am very much annoyed by the slight differences in border radius, border color, font size, padding, chevron color etc between these two inputs. They should be consistent.

👍 These should be all good now.

I noticed is when you select a value from the dropdown, the loading indicator will flash for a second. I can also see a call to the suggestions endpoint. This seems unnecessary after the user has already made a choice.

Yeah this is because it pre-fetches results as soon as we set the inputValue prop (you'll notice this if you go directly into the edit filter flow too, it searches for suggestions before you even enter the field) - there's not a whole lot I could do about it, we could potentially add some styles to hide the loader when someone makes a selection from the menu, but I think having the results pre-fetched is more often beneficial than worse.

Personally I don't like the focus style on the dropdown (in light mode, at least). I think it's bg-indigo-300 with dark text. I've created a JS dropdown for the invitations UI and I used bg-indigo-600 text-white as the hover/focus style which I think fits much better. Thoughts?

Yeah I like the styles you chose there (it's indigo 500 as per your current PR for it, so I just used that).

I did a trial with downshift instead of react-select and it seems pretty neat. Small win in bundle size but the big win I think is that it's headless so we have full control over the DOM and we can use Tailwind directly for rendering instead of !important classes.

Yeah it looks interesting - I hadn't seen it before. It might be nice, but also maybe something to look into down the line once there's some feedback on this version, and we can decide if re-implementing it is time well spent.

[ukutaht]

Thanks @Vigasaurus! Putting this on staging for further testing

[metmarkosaric]

It's so much better now, great work @Vigasaurus! loads very fast too! few notes from me here:

• Could you try to remove the word "add" from the button to make it smaller so it takes less space?

• Is it possible to add keyboard shortcut for filter popup to show up? Perhaps F?

• Can you make Page the default filter that shows up to save people a click? that first click to "select a filter" might become too cumbersome for people that use this often

• We should add info on how to search better for pages with those * and other tricks in order to for instance exclude all pages with blog in them. right now for instance i don't know how to get that result without digging through our github. there's a lot of space underneath the page filter to place a trick or two

• If I search for germany it gives me error not found, i need to search for Germany with capital G. this case sensitivity doesn't seem to be happening on other filters, it's only a problem in countries

• like discussed earlier, for second version of this, i think we should consider to merge source+referrer, browser+browser versions, os+os version to make it more user friendly

[ukutaht]

Good points @metmarkosaric. I'll work on these