fix: combined filter dropdown padding and colors.

[crhallberg]

From discussions on #3093. Thanks to @sturkel89!

[demiankatz]

Thanks, @crhallberg!

@sturkel89, when time permits, do you mind taking a look and confirming that the issues you reported are now corrected in your test environment?

[sturkel89]

I identified two issues in PR 3093: the padding and alignment of the "4 filters" button in two of the themes, and alignment of a bumped-down filter that flows onto a second line.

The "4 filters" button looks good! Here are screenshots of all three versions in the test branch:

Bootstrap3

Bootprint3

Sandal

My new nitpick (!!) is that in the first two themes, the "4 filters" button looks exactly like the Find button, but in Sandal I notice that the "4 filters" button has a drop shadow. (This is also there in the dev branch; I just hadn't noticed it because my eye focused on the alignment issue.)

I kind of like the idea of the drop shadow, as it suggests that there are a pile of filters under the button, but it's probably better to have it match the Find button -- so, @crhallberg, could you remove the shadow? :)

---

It looks like my second issue hasn't been addressed. It's very minor, so I'd want someone else (@demiankatz ?) to weigh in on whether it's worth doing. This is the observation that the second row of filters is indented just half a space from the filters margin, rather than flush left or indented a significant amount. Here's how it looks in each theme:

I think the best solution here would be aligning the bumped-down filters with the left margin of the list of filters, so right under the filter label Author: in this case. Thoughts?

[demiankatz]

Thanks for the review and clarification, @sturkel89.

It is a bit mysterious that we have a drop shadow on the combined filters in sandal only. I agree that consistency would probably be preferable, though I can live with this as is if it's complicated to fix, or if fixing it creates other inconsistencies. (That is, if we have a specific style to add a drop-shadow to this specific button in the sandal theme, I'd be in favor of removing that... but if removing that drop shadow would affect lots of other controls as well, I think we should leave it alone and take time to review the implications/consequences).

Regarding the identation, I suspect there's probably a standard left margin on all the filters, designed to even out the spacing between the filters and the boolean operators. It may not be easy to get rid of the left margin you describe without messing up the spacing of the filters and the operators (or maybe it is easy -- without looking at the code, I'm only guessing). I'd be interested to know if you reduced the browser window so that the second line started with "OR" instead of a filter whether or not you'd see the same indentation issue. I think it would be best for the indentation to be consistent whether it's a filter that wraps or an operator... but it doesn't personally bother me much whether or not any indentation shows up on the second line (I think you could make valid arguments for or against).

[sturkel89]

@crhallberg, @demiankatz and I looked at this on vufind.org/demo this morning -- here are some new comments.

1. I agree that the drop shadow fix is in the "would be nice" category -- if it's very time-consuming or breaks other things, it's probably fine to leave it as is.

2. We artificially created a really long author name to bump the connector to the second line. It's aligned with the left margin of that area! See the little AND in the screenshot below. So, it might be best to align everything at that point, with no extra indentation.

[sturkel89]

I checked out the new branch in all three themes.

1. You fixed the drop shadow!! Hooray!

2. You fixed the indent, so everything is aligned with the left margin of the filter area! Yayyyy!

My ONLY remaining nitpick is that spacing/padding is now quite tight above, below, and to the sides of each word in the filters area beneath the search box. I think more padding would be easier on the eyes. It's the same effect in all three themes, so I just have screenshots from bootprint3:

Dev branch

Test branch

I think it's fine for the spacing within a given filter to be tighter between the filter label and the filter names and the AND or OR connectors.

However,I think it would be better to have a) more vertical padding everywhere, and b) more padding between filter areas that end up on the same line, e.g., between Language: Arabic and Institution: MyInstitution.

Thanks for considering!

[demiankatz]

Thanks, @crhallberg -- your latest change seems to have addressed @sturkel89's comment about extra spacing between different filter types, but not the vertical spacing issue. I just made a minor adjustment to address that part, and I think it looks better now. A little tighter than the current dev branch, but not quite as cramped as before.

@sturkel89, could you check out the latest version here and let us know what you think of it?

[sturkel89]

Horizontal spacing looks much better! Spacing between different filter types is excellent - it's clear that these are separate from one another. This change improves upon the Dev branch. (If you could add one more pixel around the connectors within a given filter type, I think that would be a little better. But I can live with the way it is right now.)

However, I think we still need more vertical space between lines. The test branch is still very tight. Can we try it with the same vertical space as in Dev, or maybe a little bit less but further in that direction?

TEST branch:

DEV branch:

---

NB: the "4 filters" box is going to be taller than any individual filter's box, so we definitely need more vertical space than we have now in the test branch. Can the presence of that extra-tall box force a bit more padding above and below when it's in place? It looks like the vertical padding around the row with that tall box is no larger than the vertical padding around the rest of the rows. Here are a couple of quick examples. I'll show Bootprint3 because it's the only one that has a centered and not weirdly aligned or overly tall 4 filters box.

TEST branch:

DEV branch:

Thanks for indulging all of my nitpicking! @crhallberg @demiankatz

[demiankatz]

@sturkel89, I don't feel confident messing with the spacing around the operators, but I was able to add one more pixel of vertical padding as you requested, so I think that looks a bit more consistent with the dev branch now. Could you give this one (hopefully last) look?

[sturkel89]

Demian and I discussed a bit offline, and I persuaded him to add an additional pixel in vertical height. NOW the spacing looks great in all directions! 😄

This PR is ready to merge. 😄

[demiankatz]

Thanks, @sturkel89 and @crhallberg -- merging now.