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

Make minor tweaks to styles for improved Bootstrap 5 compatibility. #3261

Merged
merged 2 commits into from Dec 18, 2023

Conversation

EreMaijala
Copy link
Contributor

@EreMaijala EreMaijala commented Dec 13, 2023

From #3222.

Notes:

  • bootprint3 pagination is changed to flex as that's what Bootstrap 5 uses as well, and the table style clashed with that.
  • bootprint3 font size base setting was unnecessary, and could cause trouble with relative sizes.
  • sandal navbar selector is just a bit more specific to ensure it overrides Bootstrap styles.

@demiankatz
Copy link
Member

This looks fine to me based on a quick skim, but @crhallberg is better positioned to comment on style-specific concerns.

@sturkel89, do you mind giving this a quick test? It should look exactly the same as the dev branch -- it's just improving forward-compatibility with the newer version of bootstrap. The areas primarily impacted are the drop-down buttons used for theme/language selection in the top bar, and the pagination controls used by search results. If those look the same in all three themes, it's probably fine. (There's also a general font size simplification which shouldn't have any effect, but if you notice weird font sizes anywhere, that would be the other thing to report).

@demiankatz demiankatz added this to the 10.0 milestone Dec 14, 2023
@demiankatz demiankatz added improvement small Minor changes to relatively few files labels Dec 14, 2023
Copy link
Contributor

@sturkel89 sturkel89 left a comment

Choose a reason for hiding this comment

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

I looked at this branch in all three themes and compared it with dev. Everything looks the same in test vs. dev! This PR is ready to merge.


I have a couple of observations re: the design of elements related to pagination controls and other elements in that section of the results view. I'll leave up to you whether to create another PR for either of these. Just thought I would provide my wish list!

  • Bootstrap3 and Bootprint3 have outlining and formatting in the pagination controls; I think that looks better than the un-outlined design of the pagination controls in Sandal:
    image
    image
    VS.
    image

I also think we could do something different with alignment of top controls than the current design. As it is, in each theme the alignment gets wonky when the screen is a bit narrowed. It would work better if "Results per page" was not allowed to have a line break.

(In connection with that, perhaps the "Showing n - n for search ""; query time 1.00s" line could be forced to break after the comma.)
image
image

VS.
image
image

@demiankatz demiankatz removed the request for review from crhallberg December 18, 2023 17:24
Copy link
Member

@demiankatz demiankatz left a comment

Choose a reason for hiding this comment

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

Thanks, @EreMaijala and @sturkel89! This looks good to me, so I'm going to go ahead and merge it to keep things moving forward.

Regarding @sturkel89's more general feedback, I'm interested in whether @crhallberg or others have thoughts on that! It would definitely be nice to clean up that control wrapping, but I'm not sure how feasible it is within the technical constraints of the framework.

@demiankatz demiankatz merged commit b3a520f into vufind-org:dev Dec 18, 2023
7 checks passed
@demiankatz demiankatz deleted the dev-style-tweaks branch December 18, 2023 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement small Minor changes to relatively few files
Projects
None yet
3 participants