Tweak navbar and search tabs. #4248
Conversation
- Aligns with Bootstrap 5's documented structure. - Fixes accessibility issues (e.g. bad role="tablist" in search tabs that are just links from accessibility point of view). - Reduces redundant and unnecessary css rules. - Adds margin below search tabs (bootstrap5). - Brings MyResearch dropdown menu button closer to the text for improved visual association. - Adds left and right padding to main menu entries in collapsed mode for improved looks.
|
This is another set of changes to clean up and improve the visuals of top menu and search tabs. Should be tested also with setting Authentication/enableDropdown enabled in config.ini. |
|
As far as I can see this fixes VUFIND-1729. |
As far as I can tell, VUFIND-1729 is about record tabs, but this PR addresses search tabs. Do you mean VUFIND-1730 instead? If so, do you want to assign that one to yourself and mark it in progress? |
|
@demiankatz Oh. The JIRA issues are a bit mixed up. While the description in VUFIND-1729 is about record tabs, the comments in VUFIND-1729 mention both record tabs and search tabs. But you're right, this fixed VUFIND-1730, so I'll take it. |
|
I just resolved conflicts here to be sure it's fully up to date when @sturkel89 is ready to test. |
There was a problem hiding this comment.
I reviewed this PR in Bootstrap5 and Sandal5, comparing test vs. dev. Almost everything is better in the test branch! I have a couple of questions/suggestions, so I'll submit this review as a comment rather than a straight 'approve.'
Bootstrap5
The test branch is better than dev in all ways in Bootstrap5: font colors, padding, and alignment.
The font colors for all top menu items are all matching blue in test; in dev, the account-related ones were blue while the labels for the theme and language dropdowns were black.
The additional padding between the search tabs and the search box is an improvement. (Functionality and appearance of search tabs seems to be the same in test and dev.)
The alignment of the top menu items along the left border is better on mobile in both themes. (I'll include screenshots for Sandal5 below.) The elements are tighter in wider screen widths too.
Examples illustrating the first two improvements:
Test
Dev
Sandal5
Test is an improvement over dev in Sandal5, particularly in mobile view:
- On dev, the Your Account button overlapped the VuFind logo on mouseover; this is fixed on test.
- Left alignment of all four top menu elements is better on test than dev as well.
- (I think the heights of each element are a bit more than they need to be, though -- they seem too spaced out to me.)
- My question: the down arrow to see all the account options is missing in mobile view. Is this intentional?
At medium screen width, the top menu is hidden behind a hamburger menu, but when opened the down arrow is revealed ON ITS OWN LINE. I don't like this.
Could it be forced to stick to the Your Account button, so Your Account and Log Out take up two lines instead of three?
I appreciate the wider width of the down arrow in test; it's easier to click than the narrower version on dev. However, its alignment with the left margin is a little weird in this particular scenario. This becomes a nonissue if you can move it so it sticks right next to the Your Account button.
Test
Dev
At widest screen width, the top menu looks a little tighter and sharper in test than dev. Somehow the down arrow is wider in dev in this view. (I guess it's responsive?) I don't think you need to change anything in this view.
Test
Dev
|
@sturkel89 The dropdown wasn't supposed to be visible on the medium screen either, but since there's really no harm in making the menu always available other than it requiring a bit of tweaking to display nicely, I decided to do the required changes. So now it should be all good. The HTML structure needed changing a bit, but I think it's now better as both buttons that belong together are in the same li element. |
There was a problem hiding this comment.
Looks good! This is what I had envisioned. It works at every width, including mobile!
(I wish there were a little more color contrast between the dropdown and the rest of the background on bootstrap5, but I understand that that's supposed to be a barebones theme that the library will customize.)
TODO: