Skip to content

Template code for sidebar tabs is duplicated #1329

@osma

Description

@osma

While fixing problems with the sidebar tabs (see #1328), I found out that the template code is duplicated in no fewer than five Twig template files, with slight variations:

  • <ul class="nav nav-tabs{% if vocab.config.showChangeList and vocab.config.groupClassURI %} nav-four-wide{% endif %}">
    {% set css_class = ['nav-link'] %}
    {% if request.vocab.config.showAlphabeticalIndex %}
    <h3 class="sr-only">{% trans "List vocabulary concepts alphabetically" %}</h3>
    <li id="alpha" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/index{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% if vocab.config.showChangeList and vocab.config.groupClassURI %}{% trans 'A-Z' %}{% else %}{% trans "Alpha-nav" %}{% endif %}</a></li>
    {% endif %}
    <h3 class="sr-only">{% trans "List vocabulary concepts hierarchically" %}</h3>
    {% if active_tab == 'hierarchy' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="hierarchy{% if not vocab.config.showHierarchy %}-disabled{% endif %}" class="nav-item"><a class="{{ css_class|join(' ') }}" href="#" id="hier-trigger"{% if not vocab.config.showHierarchy %} title="{% trans 'hierarchy-disabled-help' %}"{% endif %}>{% trans "Hier-nav" %}</a></li>
    {% if vocab.config.groupClassURI %}
    {% set css_class = ['nav-link'] %}
    <h3 class="sr-only">{% trans "List vocabulary concepts and groupings hierarchically" %}</h3>
    <li id="groups" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/groups{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Group-nav" %}</a></li>
    {% endif %}
    {% if vocab.config.showChangeList %}
    {% if vocab.config.showDeprecatedChanges %}
    {% set css_class = ['nav-link'] %}
    {% set css_class = css_class|merge(['active']) %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions including removed" %}</h3>
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-and-deprecations-nav" %}</a></li>
    {% else %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions" %}</h3>
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-nav" %}</a></li>
    {% endif %}
    {% endif %}
    </ul>
  • <div class="sidebar-buttons">
    <h2 class="sr-only">{% trans "Sidebar listing: list and traverse vocabulary contents by a criterion" %}</h2>
    <ul class="nav nav-tabs{% if vocab.config.showChangeList and vocab.config.groupClassURI and vocab.config.showAlphabeticalIndex %} nav-four-wide{% endif %}">
    {% set css_class = ['nav-link'] %}
    {% if request.vocab.config.showAlphabeticalIndex %}
    <h3 class="sr-only">{% trans "List vocabulary concepts alphabetically" %}</h3>
    <li id="alpha" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/index{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% if vocab.config.showChangeList and vocab.config.groupClassURI %}{% trans 'A-Z' %}{% else %}{% trans "Alpha-nav" %}{% endif %}</a></li>
    {% endif %}
    <h3 class="sr-only">{% trans "List vocabulary concepts hierarchically" %}</h3>
    <li id="hierarchy{% if not vocab.config.showHierarchy %}-disabled{% endif %}" class="nav-item"><a class="{{ css_class|join(' ') }}" href="#" id="hier-trigger"{% if not vocab.config.showHierarchy %} title="{% trans 'hierarchy-disabled-help' %}"{% endif %}>{% trans "Hier-nav" %}</a></li>
    {% if vocab.config.groupClassURI %}
    <h3 class="sr-only">{% trans "List vocabulary concepts and groupings hierarchically" %}</h3>
    {% if search_results|first.isGroup %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="groups" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/groups{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Group-nav" %}</a></li>
    {% endif %}
    {% if vocab.config.showChangeList %}
    {% if vocab.config.showDeprecatedChanges %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions including removed" %}</h3>
    <li id="changes"><a href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-and-deprecations-nav" %}</a></li>
    {% else %}
    {% set css_class = ['nav-link'] %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions" %}</h3>
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-nav" %}</a></li>
    {% endif %}
    {% endif %}
    </ul>
    </div>
  • <div class="sidebar-buttons">
    <h2 class="sr-only">{% trans "Sidebar listing: list and traverse vocabulary contents by a criterion" %}</h2>
    <ul class="nav nav-tabs{% if vocab.config.showChangeList and vocab.config.groupClassURI and vocab.config.showAlphabeticalIndex %} nav-four-wide{% endif %}">
    {% set css_class = ['nav-link'] %}
    {% if request.vocab.config.showAlphabeticalIndex %}
    <h3 class="sr-only">{% trans "List vocabulary concepts alphabetically" %}</h3>
    <li id="alpha" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/index{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% if vocab.config.showChangeList and vocab.config.groupClassURI %}{% trans 'A-Z' %}{% else %}{% trans "Alpha-nav" %}{% endif %}</a></li>
    {% endif %}
    <h3 class="sr-only">{% trans "List vocabulary concepts hierarchically" %}</h3>
    <li id="hierarchy{% if not vocab.config.showHierarchy %}-disabled{% endif %}" class="nav-item"><a class="{{ css_class|join(' ') }}" href="#" id="hier-trigger"{% if not vocab.config.showHierarchy %} title="{% trans 'hierarchy-disabled-help' %}"{% endif %}>{% trans "Hier-nav" %}</a></li>
    {% if vocab.config.groupClassURI %}
    <h3 class="sr-only">{% trans "List vocabulary concepts and groupings hierarchically" %}</h3>
    {% set css_class = css_class|merge(['active']) %}
    <li id="groups" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/groups{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Group-nav" %}</a></li>
    {% endif %}
    {% if vocab.config.showChangeList %}
    {% if vocab.config.showDeprecatedChanges %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions including removed" %}</h3>
    <li id="changes"><a href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-and-deprecations-nav" %}</a></li>
    {% else %}
    {% set css_class = ['nav-link'] %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions" %}</h3>
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-nav" %}</a></li>
    {% endif %}
    {% endif %}
    </ul>
    </div>
  • Skosmos/view/light.twig

    Lines 44 to 86 in 290296c

    <div class="sidebar-buttons">
    {% set sidebarHeading = request.page != 'search' ? "Sidebar listing: list and traverse vocabulary contents by a criterion" : "Sidebar listing: vocabulary search options" %}
    <h2 class="sr-only">{% trans sidebarHeading %}</h2>
    <ul class="nav nav-tabs{% if vocab.config.showChangeList and vocab.config.groupClassURI and vocab.config.showAlphabeticalIndex %} nav-four-wide{% endif %}">
    {% if request.page != 'search' %}
    {% set css_class = ['nav-link'] %}
    {% if request.vocab.config.showAlphabeticalIndex %}
    <h3 class="sr-only">{% trans "List vocabulary concepts alphabetically" %}</h3>
    {% if search_results|length != 1 or (term and search_results|length == 1) %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="alpha" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/index{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% if vocab.config.showChangeList and vocab.config.groupClassURI %}{% trans'A-Z'%}{% else %}{% trans "Alpha-nav" %}{% endif %}</a></li>
    {% endif %}
    {% set css_class = ['nav-link'] %}
    {% set disabledHierarchy = not vocab.config.showHierarchy and not search_results|length == 1 %}
    <h3 class="sr-only">{% trans "List vocabulary concepts hierarchically" %}</h3>
    {% if search_results|length == 1 and not term %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="hierarchy{%- if disabledHierarchy %}-disabled{% endif -%}" class="nav-item">
    <a class="{{ css_class|join(' ') }}" href="#" id="hier-trigger"
    {% if disabledHierarchy%} title="{% trans 'hierarchy-disabled-help' %}"{% endif %}
    >{% trans "Hier-nav" %}
    </a>
    </li>
    {% set css_class = ['nav-link'] %}
    {% if vocab.config.groupClassURI %}
    <h3 class="sr-only">{% trans "List vocabulary concepts and groupings hierarchically" %}</h3>
    <li id="groups" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/groups{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Group-nav" %}</a></li>
    {% endif %}
    {% if vocab.config.showChangeList %}
    {% if vocab.config.showDeprecatedChanges %}
    {% if active_tab == 'new' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions including removed" %}</h3>
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-and-deprecations-nav" %}</a></li>
    {% else %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions" %}</h3>
    {% set css_class = ['nav-link'] %}
    {% if active_tab == 'new' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-nav" %}</a></li>
    {% endif %}
    {% endif %}
    {% else %}
    <h3 class="sr-only">{% trans "Search options tab" %}</h3>
    <li id="limit" class="nav-item"><p>{% trans "Search options" %}</p></li>
    {% endif %}
    </ul>
  • Skosmos/view/vocab.twig

    Lines 6 to 37 in 290296c

    <div class="sidebar-buttons">
    <h2 class="sr-only">{% trans "Sidebar listing: list and traverse vocabulary contents by a criterion" %}</h2>
    <ul class="nav nav-tabs{% if vocab.config.showChangeList and vocab.config.groupClassURI and vocab.config.showAlphabeticalIndex %} nav-four-wide{% endif %}">
    {% set css_class = ['nav-link'] %}
    {% if request.vocab.config.showAlphabeticalIndex %}
    <h3 class="sr-only">{% trans "List vocabulary concepts alphabetically" %}</h3>
    {% if active_tab == 'alphabetical' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="alpha" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/index{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% if vocab.config.showChangeList and vocab.config.groupClassURI %}{% trans 'A-Z' %}{% else %}{% trans "Alpha-nav" %}{% endif %}</a></li>
    {% endif %}
    <h3 class="sr-only">{% trans "List vocabulary concepts hierarchically" %}</h3>
    {% set css_class = ['nav-link'] %}
    {% if active_tab == 'hierarchy' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="hierarchy{% if not vocab.config.showHierarchy %}-disabled{% endif %}" class="nav-item"><a class="{{ css_class|join(' ') }}" href="#" id="hier-trigger"{% if not vocab.config.showHierarchy %} title="{% trans 'hierarchy-disabled-help' %}"{% endif %}>{% trans "Hier-nav" %}</a></li>
    {% if vocab.config.groupClassURI %}
    <h3 class="sr-only">{% trans "List vocabulary concepts and groupings hierarchically" %}</h3>
    <li id="groups" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/groups{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Group-nav" %}</a></li>
    {% endif %}
    {% if vocab.config.showChangeList %}
    {% if vocab.config.showDeprecatedChanges %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions including removed" %}</h3>
    {% set css_class = ['nav-link'] %}
    {% if active_tab == 'new' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-and-deprecations-nav" %}</a></li>
    {% else %}
    <h3 class="sr-only">{% trans "List vocabulary concepts by newest additions" %}</h3>
    {% set css_class = ['nav-link'] %}
    {% if active_tab == 'new' %}{% set css_class = css_class|merge(['active']) %}{% endif %}
    <li id="changes" class="nav-item"><a class="{{ css_class|join(' ') }}" href="{{ request.vocabid }}/{{ request.lang }}/new{% if request.contentLang != request.lang %}?clang={{ request.contentLang }}{% endif %}">{% trans "Changes-nav" %}</a></li>
    {% endif %}
    {% endif %}
    </ul>
    </div>

Code duplication is bad for many reasons. These should really be consolidated into just one place (for example, a separate template file called tabs.twig or sidebar-tabs.twig) but the variations need to be ironed out making sure that functionality stays the same or improves.

Metadata

Metadata

Assignees

No one assigned

    Labels

    maintenanceDependency changes, security updates, infrastructure tweaks & general mainenance

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions