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

Combined: Fix column widths in grid-layout #3232

Merged

Conversation

maccabeelevine
Copy link
Member

Fixed from #3220.

This addresses two issues with the CSS grid layout for combined search added in #3077 that occur when one column happens to have a value that can't be wrapped, and is longer than the intended width of the column. (The intention is that each column have equal width, as with the other combined column modes.) This happens in practice with EDS data that has a long, visible URL.

  1. When one column is forced longer than expected, but the combined width of all columns is still small enough to fit in .mainbody. The result is the columns are uneven.

image

  1. When once column is forced so large that the total columns can't fit in .mainbody. Then the sidebar overlaps the main content. This example is fake but I've seen it happen for real.

image

@@ -427,6 +427,10 @@ body.rtl {
.combined-search-container.grid {
margin-left: 1rem;
margin-right: 1rem;

.combined-list {
overflow-wrap: anywhere;
Copy link
Member Author

Choose a reason for hiding this comment

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

I also agree that the overflow scrollbar is not good. I'd prefer hidden but there's more we can do, I think. Since this happens when long words come into play, we can allow for more breaks in words:

.combined-list {
    word-break: break-word;
    hyphens: auto;
}

We can also do what we do locally at Villanova and make the container on the combined screen full width.

@crhallberg Yes this is so much better than a scrollbar, thanks for the idea. MDN said word-break: break-word is deprecated so I used its suggested replacement. Overflow-wrap: break-word didn't work either but :anywhere did. The hyphens rule didn't affect it, but actually given that what is wrapping (mostly) are URLs, I'm happier not hyphenating them as I think that confuses things further.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh! I hadn't realized that! Thank you for the heads up!

@maccabeelevine maccabeelevine marked this pull request as ready for review November 17, 2023 22:04
@demiankatz demiankatz merged commit 26e9336 into vufind-org:release-9.1 Nov 21, 2023
7 checks passed
@maccabeelevine maccabeelevine deleted the combined-grid-width-2 branch November 22, 2023 13:35
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.

3 participants