From a41c8f73adf11cf3394c28e1eddbb2ad0917e7af Mon Sep 17 00:00:00 2001 From: Colin Darie Date: Mon, 30 Oct 2023 15:37:04 +0100 Subject: [PATCH] refactor(nav): organize main navigation logic across profiles --- ...instructeur_expert_navigation_component.rb | 26 ++++++ ...teur_expert_navigation_component.html.haml | 12 +++ .../_main_navigation.html.haml | 5 ++ .../procedures/_main_menu.html.haml | 6 -- .../procedures/index.html.haml | 2 - app/views/layouts/_header.haml | 44 +++------- app/views/layouts/all.html.haml | 4 +- app/views/layouts/header/_avis_tab.html.haml | 5 -- .../release_notes/_main_navigation.html.haml | 27 +++--- app/views/users/_main_navigation.html.haml | 8 ++ config/locales/en.yml | 1 - config/locales/fr.yml | 1 - ...ucteur_expert_navigation_component_spec.rb | 85 +++++++++++++++++++ spec/system/accessibilite/wcag_usager_spec.rb | 28 ++++-- 14 files changed, 185 insertions(+), 69 deletions(-) create mode 100644 app/components/main_navigation/instructeur_expert_navigation_component.rb create mode 100644 app/components/main_navigation/instructeur_expert_navigation_component/instructeur_expert_navigation_component.html.haml create mode 100644 app/views/administrateurs/_main_navigation.html.haml delete mode 100644 app/views/administrateurs/procedures/_main_menu.html.haml delete mode 100644 app/views/layouts/header/_avis_tab.html.haml create mode 100644 app/views/users/_main_navigation.html.haml create mode 100644 spec/components/main_navigation/instructeur_expert_navigation_component_spec.rb diff --git a/app/components/main_navigation/instructeur_expert_navigation_component.rb b/app/components/main_navigation/instructeur_expert_navigation_component.rb new file mode 100644 index 000000000..8e8cddd41 --- /dev/null +++ b/app/components/main_navigation/instructeur_expert_navigation_component.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class MainNavigation::InstructeurExpertNavigationComponent < ApplicationComponent + def instructeur? + helpers.instructeur_signed_in? + end + + def expert? + helpers.expert_signed_in? + end + + def aria_current_for(page) + { current: page == current_page ? :page : nil } + end + + private + + def current_page + case controller_name + when 'avis' + :avis + when 'procedures', 'dossiers' + :procedure + end + end +end diff --git a/app/components/main_navigation/instructeur_expert_navigation_component/instructeur_expert_navigation_component.html.haml b/app/components/main_navigation/instructeur_expert_navigation_component/instructeur_expert_navigation_component.html.haml new file mode 100644 index 000000000..411163861 --- /dev/null +++ b/app/components/main_navigation/instructeur_expert_navigation_component/instructeur_expert_navigation_component.html.haml @@ -0,0 +1,12 @@ +%nav#header-navigation.fr-nav{ role: :navigation, "aria-label" => t('main_menu', scope: [:layouts, :header]) } + %ul.fr-nav__list + - if instructeur? + %li.fr-nav__item + = link_to Procedure.model_name.human(count: 10), instructeur_procedures_path, class: 'fr-nav__link', aria: aria_current_for(:procedure) + + - if expert? + %li.fr-nav__item + = link_to expert_all_avis_path, class: 'fr-nav__link', aria: aria_current_for(:avis) do + = Avis.model_name.human(count: 10) + - if helpers.current_expert.avis_summary[:unanswered] > 0 + %span.badge.warning= helpers.current_expert.avis_summary[:unanswered] diff --git a/app/views/administrateurs/_main_navigation.html.haml b/app/views/administrateurs/_main_navigation.html.haml new file mode 100644 index 000000000..04693d235 --- /dev/null +++ b/app/views/administrateurs/_main_navigation.html.haml @@ -0,0 +1,5 @@ +%nav#header-navigation.fr-nav{ role: 'navigation', 'aria-label': 'Menu principal administrateur' } + %ul.fr-nav__list + %li.fr-nav__item= link_to 'Mes démarches', admin_procedures_path, class:'fr-nav__link', 'aria-current': current_page?(controller: 'administrateurs/procedures', action: :index) ? 'true' : nil + - if Rails.application.config.ds_zonage_enabled + %li.fr-nav__item= link_to 'Toutes les démarches', all_admin_procedures_path(zone_ids: current_administrateur.zones), class:'fr-nav__link', 'aria-current': current_page?(all_admin_procedures_path) ? 'page' : nil diff --git a/app/views/administrateurs/procedures/_main_menu.html.haml b/app/views/administrateurs/procedures/_main_menu.html.haml deleted file mode 100644 index 89f3de634..000000000 --- a/app/views/administrateurs/procedures/_main_menu.html.haml +++ /dev/null @@ -1,6 +0,0 @@ -.fr-container - %nav#header-navigation.fr-nav{ role: 'navigation', 'aria-label': 'Menu principal administrateur' } - %ul.fr-nav__list - %li.fr-nav__item= link_to 'Mes démarches', admin_procedures_path, class:'fr-nav__link', 'aria-current': current_page?(controller: 'procedures', action: :index) ? 'true' : nil - - if Rails.application.config.ds_zonage_enabled - %li.fr-nav__item= link_to 'Toutes les démarches', all_admin_procedures_path(zone_ids: current_administrateur.zones), class:'fr-nav__link', 'aria-current': current_page?(all_admin_procedures_path) ? 'page' : nil diff --git a/app/views/administrateurs/procedures/index.html.haml b/app/views/administrateurs/procedures/index.html.haml index 8c17eddd0..60d2441da 100644 --- a/app/views/administrateurs/procedures/index.html.haml +++ b/app/views/administrateurs/procedures/index.html.haml @@ -1,5 +1,3 @@ -= render 'main_menu' - .sub-header .procedure-admin-listing-container = link_to "Nouvelle Démarche", new_from_existing_admin_procedures_path, id: 'new-procedure', class: 'fr-btn' diff --git a/app/views/layouts/_header.haml b/app/views/layouts/_header.haml index 22a4e47e3..07425860d 100644 --- a/app/views/layouts/_header.haml +++ b/app/views/layouts/_header.haml @@ -3,6 +3,7 @@ - dossier = controller.try(:dossier_for_help) - procedure = controller.try(:procedure_for_help) - is_instructeur_context = nav_bar_profile == :instructeur && instructeur_signed_in? +- is_administrateur_context = nav_bar_profile == :administrateur && administrateur_signed_in? - is_expert_context = nav_bar_profile == :expert && expert_signed_in? - is_user_context = nav_bar_profile == :user - is_search_enabled = [params[:controller] == 'recherche', is_instructeur_context, is_expert_context, is_user_context && current_user.dossiers.count].any? @@ -20,7 +21,7 @@ .fr-header__navbar - if is_search_enabled %button.fr-btn--search.fr-btn{ "aria-controls" => "search-modal", "data-fr-opened" => "false", :title => t('views.users.dossiers.search.search_file') }= t('views.users.dossiers.search.search_file') - %button.fr-btn--menu.fr-btn{ "aria-controls" => "burger-menu", "aria-haspopup" => "menu", "data-fr-opened" => "false", :title => "Menu" } Menu + %button#navbar-burger-button.fr-btn--menu.fr-btn{ "aria-controls" => "modal-header__menu", "aria-haspopup" => "menu", "data-fr-opened" => "false", title: "Menu" } Menu .fr-header__service - root_profile_link, root_profile_libelle = root_path_info_for_profile(nav_bar_profile) @@ -68,39 +69,20 @@ - if is_expert_context = render partial: 'layouts/search_dossiers_form' - - has_header = [is_instructeur_context, is_expert_context, is_user_context] - #burger-menu.fr-header__menu.fr-modal + #modal-header__menu.fr-header__menu.fr-modal{ "aria-labelledby": "navbar-burger-button" } .fr-container - %button#burger_button.fr-btn--close.fr-btn{ "aria-controls" => "burger-menu", :title => t('close_modal', scope: [:layouts, :header]) }= t('close_modal', scope: [:layouts, :header]) + %button.fr-btn--close.fr-btn{ "aria-controls" => "modal-header__menu", title: t('close_modal', scope: [:layouts, :header]) }= t('close_modal', scope: [:layouts, :header]) .fr-header__menu-links - %nav#navigation-478.fr-nav{ "aria-label" => t('main_menu', scope: [:layouts, :header]) , :role => "navigation" } - %ul.fr-nav__list - -# Questionner UX pour un back JS - - if params[:controller] == 'users/commencer' - %li.fr-nav__item - = link_to t('back', scope: [:layouts, :header]), url_for(:back), title: t('back_title', scope: [:layouts, :header]), class: 'fr-nav__link' + -# populated by dsfr js - - if is_instructeur_context - - if current_instructeur.procedures.any? - - current_url = request.path_info - %li.fr-nav__item - = active_link_to t('utils.procedure'), instructeur_procedures_path, active: ['dossiers','procedures'].include?(controller_name), class: 'fr-nav__link', aria: { current: current_url == instructeur_procedures_path ? 'page' : true } - - if current_instructeur.user.expert && current_expert.avis_summary[:total] > 0 - = render partial: 'layouts/header/avis_tab', locals: { current_expert: current_expert } + - if content_for?(:main_navigation) + = yield(:main_navigation) + - elsif is_administrateur_context + = render 'administrateurs/main_navigation' + - elsif is_instructeur_context || is_expert_context + = render MainNavigation::InstructeurExpertNavigationComponent.new + - elsif is_user_context + = render 'users/main_navigation' - - if is_expert_context - - if current_expert.user.instructeur && current_instructeur.procedures.any? - %li.fr-nav__item= active_link_to t('utils.procedure'), instructeur_procedures_path, active: ['dossiers','procedures'].include?(controller_name), class: 'fr-nav__link', aria: { current: true } - - if current_expert.avis_summary[:total] > 0 - = render partial: 'layouts/header/avis_tab', locals: { current_expert: current_expert } - - - if is_user_context - %li.fr-nav__item= active_link_to t('.files'), dossiers_path, active: :inclusive, class: 'fr-nav__link', aria: { current: true } - - if current_user.expert && current_expert.avis_summary[:total] > 0 - = render partial: 'layouts/header/avis_tab', locals: { current_expert: current_expert } - - - if content_for?(:navigation_principale) - .fr-container - = yield(:navigation_principale) = yield(:notice_info) diff --git a/app/views/layouts/all.html.haml b/app/views/layouts/all.html.haml index 065cf64eb..bb581b79e 100644 --- a/app/views/layouts/all.html.haml +++ b/app/views/layouts/all.html.haml @@ -1,5 +1,7 @@ +- content_for(:main_navigation) do + = render 'administrateurs/main_navigation' + - content_for :content do - = render 'main_menu' .fr-container %h1.fr-my-4w Toutes les démarches diff --git a/app/views/layouts/header/_avis_tab.html.haml b/app/views/layouts/header/_avis_tab.html.haml deleted file mode 100644 index 91031da4d..000000000 --- a/app/views/layouts/header/_avis_tab.html.haml +++ /dev/null @@ -1,5 +0,0 @@ -%li.fr-nav__item - = active_link_to expert_all_avis_path, active: controller_name == 'avis', class: 'fr-nav__link' do - Avis - - if current_expert.avis_summary[:unanswered] > 0 - %span.badge.warning= current_expert.avis_summary[:unanswered] diff --git a/app/views/super_admins/release_notes/_main_navigation.html.haml b/app/views/super_admins/release_notes/_main_navigation.html.haml index 2959495b0..8207cee48 100644 --- a/app/views/super_admins/release_notes/_main_navigation.html.haml +++ b/app/views/super_admins/release_notes/_main_navigation.html.haml @@ -1,17 +1,16 @@ -- content_for(:navigation_principale) do - .fr-container - %nav.fr-nav#header-navigation{ role: "navigation", aria: { label: 'Menu principal annonces' } } - %ul.fr-nav__list +- content_for(:main_navigation) do + %nav.fr-nav#header-navigation{ role: "navigation", aria: { label: 'Menu principal annonces' } } + %ul.fr-nav__list + %li.fr-nav__item + = link_to "Toutes les annonces", super_admins_release_notes_path, class: "fr-nav__link", target: "_self", aria: { current: action == :index ? "page" : nil } + + %li.fr-nav__item + = link_to("Nouvelle annonce", new_super_admins_release_note_path(date: @release_note&.released_on), class: "fr-nav__link", target: "_self", aria: { current: action == :new ? "page" : nil }) + + - if action == :edit %li.fr-nav__item - = link_to "Toutes les annonces", super_admins_release_notes_path, class: "fr-nav__link", target: "_self", aria: { current: action == :index ? "page" : nil } + = link_to "Annonce", '', class: "fr-nav__link", target: "_self", aria: { current: "page" } - %li.fr-nav__item - = link_to("Nouvelle annonce", new_super_admins_release_note_path(date: @release_note&.released_on), class: "fr-nav__link", target: "_self", aria: { current: action == :new ? "page" : nil }) - - - if action == :edit - %li.fr-nav__item - = link_to "Annonce", '', class: "fr-nav__link", target: "_self", aria: { current: "page" } - - %li.fr-nav__item - = link_to "Annonces publiées", release_notes_path, class: "fr-nav__link", target: "_self" + %li.fr-nav__item + = link_to "Annonces publiées", release_notes_path, class: "fr-nav__link", target: "_self" diff --git a/app/views/users/_main_navigation.html.haml b/app/views/users/_main_navigation.html.haml new file mode 100644 index 000000000..602dfec9f --- /dev/null +++ b/app/views/users/_main_navigation.html.haml @@ -0,0 +1,8 @@ +%nav#header-navigation.fr-nav{ role: :navigation, "aria-label" => t('main_menu', scope: [:layouts, :header]) } + %ul.fr-nav__list + - if params[:controller] == 'users/commencer' + %li.fr-nav__item + = link_to t('back', scope: [:layouts, :header]), url_for(:back), title: t('back_title', scope: [:layouts, :header]), class: 'fr-nav__link', "aria-controls" => "modal-header__menu" + + %li.fr-nav__item + = link_to t('files', scope: [:layouts, :header]), dossiers_path, class: 'fr-nav__link', aria: { current: current_page?(dossiers_path) ? 'page' : nil, controls: "modal-header__menu" } diff --git a/config/locales/en.yml b/config/locales/en.yml index 477434739..8b97107cb 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -56,7 +56,6 @@ en: subject: Subject message: Message send_mail: Send message - procedure: Procedures new_tab: New tab helpers: procedure: diff --git a/config/locales/fr.yml b/config/locales/fr.yml index a54e8d379..ace408772 100644 --- a/config/locales/fr.yml +++ b/config/locales/fr.yml @@ -47,7 +47,6 @@ fr: subject: Sujet message: Message send_mail: Envoyer le message - procedure: Démarches new_tab: "Nouvel onglet" helpers: procedure: diff --git a/spec/components/main_navigation/instructeur_expert_navigation_component_spec.rb b/spec/components/main_navigation/instructeur_expert_navigation_component_spec.rb new file mode 100644 index 000000000..ea743c2df --- /dev/null +++ b/spec/components/main_navigation/instructeur_expert_navigation_component_spec.rb @@ -0,0 +1,85 @@ +describe MainNavigation::InstructeurExpertNavigationComponent, type: :component do + let(:component) { described_class.new } + let(:as_instructeur) { true } + let(:as_expert) { false } + let(:controller_name) { 'dossiers' } + let(:user) { build(:user) } + + subject { render_inline(component) } + + before do + if as_instructeur + user.build_instructeur + end + + if as_expert + user.build_expert + end + + allow_any_instance_of(ApplicationController).to receive(:current_user).and_return(user) + allow_any_instance_of(ApplicationController).to receive(:administrateur_signed_in?).and_return(false) + allow_any_instance_of(ApplicationController).to receive(:controller_name).and_return(controller_name) + end + + describe 'when instructor is signed in' do + it 'renders a link to instructeur procedures with current page class' do + expect(subject).to have_link('Démarches', href: component.helpers.instructeur_procedures_path) + expect(subject).to have_selector('a[aria-current="page"]', text: 'Démarches') + end + + it 'does not have Avis' do + expect(subject).not_to have_link('Avis') + end + + context 'when instructor is also an expert' do + let(:as_expert) { true } + before do + allow(user.expert).to receive(:avis_summary).and_return({ unanswered: 0 }) + end + + it 'render have Avis link' do + expect(subject).to have_link('Avis', href: component.helpers.expert_all_avis_path) + expect(subject).not_to have_selector('a[aria-current="page"]', text: 'Avis') + end + end + end + + describe 'when expert is signed in' do + let(:as_instructeur) { false } + let(:as_expert) { true } + + let(:unanswered) { 0 } + let(:controller_name) { 'avis' } + + before do + allow(user.expert).to receive(:avis_summary).and_return({ unanswered: }) + end + + it 'renders a link to expert all avis with current page class' do + expect(subject).to have_link('Avis', href: component.helpers.expert_all_avis_path) + expect(subject).to have_selector('a[aria-current="page"]', text: 'Avis') + expect(subject).not_to have_selector('span.badge') + end + + it 'does not have Démarches link' do + expect(subject).not_to have_link('Démarches') + end + + context 'when there are unanswered avis' do + let(:unanswered) { 2 } + + it 'renders an unanswered avis badge for the expert' do + expect(subject).to have_selector('span.badge.warning', text: '2') + end + end + + context 'when expert is also instructor' do + let(:as_instructeur) { true } + + it 'render have Démarches link' do + expect(subject).to have_link('Démarches', href: component.helpers.instructeur_procedures_path) + expect(subject).not_to have_selector('a[aria-current="page"]', text: 'Démarches') + end + end + end +end diff --git a/spec/system/accessibilite/wcag_usager_spec.rb b/spec/system/accessibilite/wcag_usager_spec.rb index 411ed2361..e9aec8277 100644 --- a/spec/system/accessibilite/wcag_usager_spec.rb +++ b/spec/system/accessibilite/wcag_usager_spec.rb @@ -27,6 +27,18 @@ describe 'wcag rules for usager', js: true do end end + def expect_axe_clean_without_main_navigation + # On page without main navigation content (like anonymous home page), + # there are either a bug in axe, either dsfr markup is not conform to wcag2a. + # There is no issue on pages having a child navigation. + expect(page).to be_axe_clean.excluding("#modal-header__menu") + expect(page).to be_axe_clean.within("#modal-header__menu").skipping("aria-prohibited-attr") + end + + shared_examples "axe clean without main navigation" do + it { expect_axe_clean_without_main_navigation } + end + context 'pages without the need to be logged in' do before do visit path @@ -34,14 +46,14 @@ describe 'wcag rules for usager', js: true do context 'homepage' do let(:path) { root_path } - it { expect(page).to be_axe_clean } + it_behaves_like "axe clean without main navigation" it_behaves_like "external links have title says it opens in a new tab" it_behaves_like "aria-label do not mix with title attribute" end context 'sign_up page' do let(:path) { new_user_registration_path } - it { expect(page).to be_axe_clean } + it_behaves_like "axe clean without main navigation" it_behaves_like "external links have title says it opens in a new tab" it_behaves_like "aria-label do not mix with title attribute" end @@ -54,11 +66,11 @@ describe 'wcag rules for usager', js: true do perform_enqueued_jobs do click_button 'Créer un compte' - expect(page).to be_axe_clean + expect_axe_clean_without_main_navigation end end - context 'sign_upc confirmation' do + context 'sign_up confirmation' do let(:path) { user_confirmation_path("user[email]" => "some@email.com") } it_behaves_like "external links have title says it opens in a new tab" @@ -67,21 +79,21 @@ describe 'wcag rules for usager', js: true do context 'sign_in page' do let(:path) { new_user_session_path } - it { expect(page).to be_axe_clean.excluding '#user_email' } + it_behaves_like "axe clean without main navigation" it_behaves_like "external links have title says it opens in a new tab" it_behaves_like "aria-label do not mix with title attribute" end context 'contact page' do let(:path) { contact_path } - it { expect(page).to be_axe_clean } + it_behaves_like "axe clean without main navigation" it_behaves_like "external links have title says it opens in a new tab" it_behaves_like "aria-label do not mix with title attribute" end context 'commencer page' do let(:path) { commencer_path(path: procedure.path) } - it { expect(page).to be_axe_clean } + it_behaves_like "axe clean without main navigation" it_behaves_like "external links have title says it opens in a new tab" it_behaves_like "aria-label do not mix with title attribute" end @@ -90,7 +102,7 @@ describe 'wcag rules for usager', js: true do visit commencer_path(path: procedure.reload.path) page.find("#help-menu_button").click - expect(page).to be_axe_clean + expect_axe_clean_without_main_navigation end end