-
Notifications
You must be signed in to change notification settings - Fork 100
Closed
Labels
maintenanceDependency changes, security updates, infrastructure tweaks & general mainenanceDependency changes, security updates, infrastructure tweaks & general mainenance
Milestone
Description
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:
Lines 7 to 32 in 290296c
<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> Skosmos/view/group-contents.twig
Lines 7 to 33 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> <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> Lines 6 to 32 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> <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> 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> 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.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
maintenanceDependency changes, security updates, infrastructure tweaks & general mainenanceDependency changes, security updates, infrastructure tweaks & general mainenance