Skip to content

Set max width for topbar and headerbar#1791

Merged
osma merged 1 commit intomainfrom
issue1480-topbar-max-width
May 21, 2025
Merged

Set max width for topbar and headerbar#1791
osma merged 1 commit intomainfrom
issue1480-topbar-max-width

Conversation

@osma
Copy link
Member

@osma osma commented May 21, 2025

Reasons for creating this PR

The layout of the top header looks funny on very wide screens, because the contents (logo, navigation links etc.) are placed near the edges of the page.

This PR changes the header elements to use Bootstrap Grid classes (row, col) and then sets a max-width of 1460px for the contents. Also some margins and paddings are adjusted. The end result is that the navigation elements at the top are kept in the middle of the page, not near the edges.

Link to relevant issue(s), if any

Description of the changes in this PR

  • use Bootstrap Grid classes for header
  • add max-width 1460px for the header elements
  • adjust margins and paddings

Known problems or uncertainties in this PR

I'm still not 100% happy with the alignment of the left and right edges. Especially on some zoom levels / page widths the edges don't always align nicely.

One problem here is that the Skosmos logo used within vocabularies (negative version, for use on a dark background) includes a lot of empty space around the logo itself within the svg file, making it hard to align. I tried two methods of cropping the svg to its content, but when I replaced the svg file with a cropped version, the logo just failed to display at all. I can't figure out why, but in any case, here are the two cropped logos (they look a bit funny here if you have a white background) in case they are useful:

skosmos-NEGA-RGB-cropped
skosmos-NEGA-RGB-cropped-2

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma self-assigned this May 21, 2025
@osma osma added this to the 3.0-alpha.2 milestone May 21, 2025
@osma osma requested a review from joelit May 21, 2025 09:09
@sonarqubecloud
Copy link

@osma osma moved this to Needs review in Skosmos 3.x Backlog May 21, 2025
Copy link
Contributor

@joelit joelit left a comment

Choose a reason for hiding this comment

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

Looks good, and it is definitely an improvement! I would align the logo to the main container using the bootstrap width breakpoints at 1400, 1200, 992, etc. so that the logo would get similar margins to the main container. Now the left-side margin of the logo is aligned to the topbar text.

However, this is down to individual taste. We could just iterate and ask for user feedback and resume tweaking the CSS on a later occasion - but then the issue needs to be documented.

The svg-logo started working by giving read permissions to it.

@osma
Copy link
Member Author

osma commented May 21, 2025

I suggest we merge this as is and then iterate later after wider review. It's a bit unclear what the requirements for the alignment and margins are.

@osma osma merged commit d5374ff into main May 21, 2025
10 checks passed
@github-project-automation github-project-automation bot moved this from Needs review to Issue/PR closed in Skosmos 3.x Backlog May 21, 2025
@osma osma deleted the issue1480-topbar-max-width branch May 21, 2025 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants