Sidebar tab styling on narrow screens by UnniKohonen · Pull Request #1806 · NatLibFi/Skosmos

UnniKohonen · 2025-08-28T07:27:42Z

Reasons for creating this PR

Sidebar tabs end up on two separate lines on narrow screen sizes. This PR adds an initial fix for this.

Link to relevant issue(s), if any

Part of Epic: Sidebar #1747

Description of the changes in this PR

Make sidebar tab padding more narrow
Add stylings for sidebars with 4 tabs:
- Reduce padding more on narrow screens
- Make last tab overflow with a fade out on narrow screens
- Use A-Z on alphabetical tab

Addresses requirement 7.2 in #1747

Known problems or uncertainties in this PR

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

The end result looks good, but I'm a bit worried about the amount of new CSS this introduces, and the !important rules that could cause problems for users.

I think there could be opportunities to use Boostrap utility classes (e.g. Spacing, Text, Overflow) in the HTML instead of CSS rules. I've suggested a few in the comments. Of course there might be some problem with these that aren't immediately obvious to me :) But please at least check if there are ways like this to simplify the CSS.

osma · 2025-08-28T08:18:04Z

resource/css/skosmos.css


+  /* In smaller screen sizes, the last sidebar tab overflows to the right */
+  @media only screen and (min-width: 768px) and (max-width: 1070px) {
+    #sidebar-tabs.wide .nav {


Does this actually match any HTML elements? It seems to me that the .nav class is set for the same element (#sidebar-tabs), so this doesn't match its children.

In any case, I think this could be replaced with class="flex-nowrap" which is a Bootstrap utility class.

I fixed the rule but it has to be done in CSS as it is only applied on narrow screens

osma · 2025-08-28T08:22:54Z

resource/css/skosmos.css

+    }
+  }
+
+  @media only screen and (max-width: 900px) {


Could these be replaced with Bootstrap's responsive padding utility classes such as px-sm-1 px-md-2 py-2 ...?

The bootstrap breakpoints aren't really granular enough (md is >768px and lg >992px screen width)

osma · 2025-08-28T08:23:08Z

resource/css/skosmos.css

+  }
+
+
+  @media only screen and (max-width: 820px) {


Ditto, use responsive padding utility classes?

I will remove this because it is not really needed

osma · 2025-08-28T08:24:28Z

resource/css/skosmos.css

+    }
+
+    #sidebar-tabs.wide .nav-item:last-of-type .nav-link {
+      justify-content: start !important;


Could this be replaced with Bootstrap's text-start utility class?

The rule is dependent on the screen width which the twig template doesn't know

osma · 2025-08-28T08:27:32Z

resource/css/skosmos.css

+    }
+
+    #sidebar-tabs.wide .nav-link {
+      padding: 0.5rem 0.25rem;


Replace with Bootstrap padding classes?

Same as above

osma · 2025-08-28T08:31:21Z

resource/css/skosmos.css

+    }
+
+    #sidebar-tabs.wide .nav-item:last-of-type {
+      overflow: hidden;


Replace with overflow-hidden class? Or is it too difficult to determine the last tab in Twig templates?

Same as above

osma · 2025-08-28T08:32:04Z

src/view/sidebar.inc.twig

    <div class="sidebar-buttons">
      <h2 class="visually-hidden">{{'Sidebar listing: list and traverse vocabulary contents by a criterion' | trans}}</h2>
-      <ul class="nav nav-tabs-no-style nav-justified" id="sidebar-tabs" role="tablist">
+      <ul class="nav nav-tabs-no-style nav-justified{% if vocab.config.sidebarViews | length == 4 and vocab.config.showDeprecatedChanges %} wide{% endif %}" id="sidebar-tabs" role="tablist">


Could we change the condition to >= 4 just in case it's later possible to add more than 4 tabs?

osma · 2025-08-28T08:32:22Z

src/view/sidebar.inc.twig

+      <ul class="nav nav-tabs-no-style nav-justified{% if vocab.config.sidebarViews | length == 4 and vocab.config.showDeprecatedChanges %} wide{% endif %}" id="sidebar-tabs" role="tablist">
        {% for view in vocab.config.sidebarViews %}
-          {% if view == 'alphabetical' %} {% set view_trans_text = 'Alpha-nav' %}
+          {% if view == 'alphabetical' %} {% set view_trans_text = (vocab.config.sidebarViews | length == 4) ? 'A-Z' : 'Alpha-nav' %}


Ditto, >= 4 would be more future-proof

osma · 2025-08-28T08:33:34Z

resource/css/skosmos.css

    align-items: center;
    justify-content: center;
    transition: none;
+    padding: 0.5rem;


Use Bootstrap utility class p-2 or something along those lines?

osma

Please merge after considering my suggestions

sonarqubecloud · 2025-08-28T09:17:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

UnniKohonen added 2 commits August 28, 2025 09:52

Add stylings for sidebar tabs on narrow screens

07c6387

Fix alphabetical tab style

2be6e86

UnniKohonen added this to the 3.0-beta.1 milestone Aug 28, 2025

UnniKohonen requested a review from osma August 28, 2025 07:27

UnniKohonen self-assigned this Aug 28, 2025

UnniKohonen added this to Skosmos 3.x Backlog Aug 28, 2025

UnniKohonen added the enhancement label Aug 28, 2025

UnniKohonen added 2 commits August 28, 2025 10:29

Fix alphabetical tab style on narrow screen

e13f5ee

Simplify last tab fade rules

52fbebb

UnniKohonen moved this to Needs review in Skosmos 3.x Backlog Aug 28, 2025

osma requested changes Aug 28, 2025

View reviewed changes

osma approved these changes Aug 28, 2025

View reviewed changes

Minor CSS fixes

d744cb1

UnniKohonen merged commit 5f34aa8 into main Aug 28, 2025
12 checks passed

github-project-automation bot moved this from Needs review to Issue/PR closed in Skosmos 3.x Backlog Aug 28, 2025

UnniKohonen deleted the narrow-sidebar-tabs-visibility branch August 28, 2025 09:50

UnniKohonen moved this from Issue/PR closed to Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR) in Skosmos 3.x Backlog Aug 28, 2025

osma removed this from Skosmos 3.x Backlog Aug 28, 2025

Conversation

UnniKohonen commented Aug 28, 2025

Reasons for creating this PR

Link to relevant issue(s), if any

Description of the changes in this PR

Known problems or uncertainties in this PR

Checklist

Uh oh!

osma left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

osma left a comment

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Aug 28, 2025

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants