Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUGFIX] Réordonnancement des fieldset legend de Modulix (PIX-12382) #8835

Merged
merged 1 commit into from
May 22, 2024
Merged

[BUGFIX] Réordonnancement des fieldset legend de Modulix (PIX-12382) #8835

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 10 additions & 18 deletions mon-pix/app/pods/components/module/qcm/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
.element-qcm {
&__header {
display: flex;
flex-direction: column-reverse;
&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}

&__direction {
@extend %pix-body-s;

color: var(--pix-neutral-500);
}

&__proposals {
margin-bottom: var(--pix-spacing-4x);
margin: var(--pix-spacing-4x) 0;
}

&__required-field-missing {
Expand All @@ -17,20 +23,6 @@
}
}

.element-qcm-header {
&__direction {
@extend %pix-body-s;

margin-bottom: var(--pix-spacing-4x);
color: var(--pix-neutral-500);
}

&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}
}

.element-qcm-proposals {
&__proposal {
padding: var(--pix-spacing-2x);
Expand Down
20 changes: 11 additions & 9 deletions mon-pix/app/pods/components/module/qcm/template.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
<form class="element-qcm">
<fieldset>
<div class="element-qcm__header">
<legend class="element-qcm-header__direction">
{{t "pages.modulix.qcm.direction"}}
</legend>
<form class="element-qcm" aria-describedby="instruction-{{this.element.id}}">
<fieldset disabled={{this.disableInput}}>
<legend class="screen-reader-only">
{{t "pages.modulix.qcm.direction"}}
</legend>

<div class="element-qcm-header__instruction">
{{html-unsafe this.element.instruction}}
</div>
<div class="element-qcm__instruction" id="instruction-{{this.element.id}}">
{{html-unsafe this.element.instruction}}
</div>

<p class="element-qcm__direction" aria-hidden="true">
{{t "pages.modulix.qcm.direction"}}
</p>

<div class="element-qcm__proposals">
{{#each this.element.proposals as |proposal|}}
<div class="element-qcm-proposals__proposal">
Expand Down
28 changes: 10 additions & 18 deletions mon-pix/app/pods/components/module/qcu/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
.element-qcu {
&__header {
display: flex;
flex-direction: column-reverse;
&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}

&__direction {
@extend %pix-body-s;

color: var(--pix-neutral-500);
}

&__proposals {
margin-bottom: var(--pix-spacing-4x);
margin: var(--pix-spacing-4x) 0;
}

&__required-field-missing {
Expand All @@ -17,20 +23,6 @@
}
}

.element-qcu-header {
&__direction {
@extend %pix-body-s;

margin-bottom: var(--pix-spacing-4x);
color: var(--pix-neutral-500);
}

&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}
}

.element-qcu-proposals {
&__proposal {
padding: var(--pix-spacing-2x);
Expand Down
20 changes: 11 additions & 9 deletions mon-pix/app/pods/components/module/qcu/template.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
<form class="element-qcu">
<fieldset>
<div class="element-qcu__header">
<legend class="element-qcu-header__direction">
{{t "pages.modulix.qcu.direction"}}
</legend>
<form class="element-qcu" aria-describedby="instruction-{{this.element.id}}">
<fieldset disabled={{this.disableInput}}>
<legend class="screen-reader-only">
{{t "pages.modulix.qcu.direction"}}
</legend>

<div class="element-qcu-header__instruction">
{{html-unsafe this.element.instruction}}
</div>
<div class="element-qcu__instruction" id="instruction-{{this.element.id}}">
{{html-unsafe this.element.instruction}}
</div>

<p class="element-qcu__direction" aria-hidden="true">
{{t "pages.modulix.qcu.direction"}}
</p>

<div class="element-qcu__proposals">
{{#each this.element.proposals as |proposal|}}
<div class="element-qcu-proposals__proposal">
Expand Down
28 changes: 10 additions & 18 deletions mon-pix/app/pods/components/module/qrocm/styles.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,17 @@
.element-qrocm {
&__header {
display: flex;
flex-direction: column-reverse;
&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}

&__direction {
@extend %pix-body-s;

color: var(--pix-neutral-500);
}

&__proposals {
margin-bottom: var(--pix-spacing-4x);
margin: var(--pix-spacing-4x) 0;
}

&__required-field-missing {
Expand All @@ -17,20 +23,6 @@
}
}

.element-qrocm-header {
&__direction {
@extend %pix-body-s;

margin-bottom: var(--pix-spacing-4x);
color: var(--pix-neutral-500);
}

&__instruction {
margin-bottom: var(--pix-spacing-2x);
font-weight: var(--pix-font-medium);
}
}

.element-qrocm-proposals {
&__input {
line-height: var(--pix-spacing-12x);
Expand Down
25 changes: 17 additions & 8 deletions mon-pix/app/pods/components/module/qrocm/template.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
<form class="element-qrocm" autocapitalize="off" autocomplete="nope" autocorrect="off" spellcheck="false">
<form
class="element-qrocm"
aria-describedby="instruction-{{this.element.id}}"
autocapitalize="off"
autocomplete="nope"
autocorrect="off"
spellcheck="false"
>
<fieldset disabled={{this.disableInput}}>
<div class="element-qrocm__header">
<legend class="element-qrocm-header__direction">
{{t "pages.modulix.qrocm.direction" count=this.nbOfProposals}}
</legend>
<legend class="screen-reader-only">
{{t "pages.modulix.qrocm.direction" count=this.nbOfProposals}}
</legend>

<div class="element-qrocm-header__instruction">
{{html-unsafe this.element.instruction}}
</div>
<div class="element-qrocm__instruction" id="instruction-{{this.element.id}}">
{{html-unsafe this.element.instruction}}
</div>

<p class="element-qrocm__direction" aria-hidden="true">
{{t "pages.modulix.qrocm.direction" count=this.nbOfProposals}}
</p>

<div class="element-qrocm__proposals">
{{#each this.formattedProposals as |block|}}
{{#if (eq block.type "text")}}
Expand Down
12 changes: 8 additions & 4 deletions mon-pix/tests/integration/components/module/qcm_test.js
View file
Open in desktop
yannbertrand marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { render } from '@1024pix/ember-testing-library';
import { click, findAll } from '@ember/test-helpers';
// eslint-disable-next-line no-restricted-imports
import { click, find } from '@ember/test-helpers';
Comment on lines +2 to +3
Copy link
Member Author

Choose a reason for hiding this comment

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

Je ne sais pas comment récupérer le formulaire pour vérifier la valeur de son aria-describedby avec testing library... Preneur de mieux !

Copy link
Contributor

Choose a reason for hiding this comment

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

Une PR est en cours sur le repo de testing-library sur le sujet.
A11yance/aria-query#547
C'est lié à ce bug-ci
testing-library/dom-testing-library#1293
D'ici là, la doc ne semble pas proposer d'alternatives qui puissent convenir.

Faute de mieux, ce que tu proposes me convient. Peut-être mettre un commentaire au-dessus de la ligne 31 avec le lien vers la PR de testing-library ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Pour info j'avais eu un process de pensée un peu différent. Sur MDN form n'a le rôle form que si il est décrit par un nom accessible, j'imagine que la complexité d'implémentation côté testing-library vient de là.

Copy link
Contributor

Choose a reason for hiding this comment

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

@yannbertrand ils entendent quoi par un nom accessible ?

Copy link
Member Author

Choose a reason for hiding this comment

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

J'ai pas lu mais la définition est linkée dans MDN, ça renvoie là : https://www.w3.org/TR/accname-1.1/#dfn-accessible-name

Copy link
Contributor

Choose a reason for hiding this comment

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

merci, c'est très clair maintenant :) https://www.w3.org/TR/accname-1.1/#dfn-accessible-name:~:text=A%20simple%20use,or%20%22OK%20button%22.

Copy link
Member Author

Choose a reason for hiding this comment

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

Encore plus clair après notre discussion (après avoir lu mdn).

Par défaut la balise form n'a pas le rôle form. Le debugger d'accessibilité Firefox ne le constate pas, mais c'est bien visible sur Chrome qui annonce un role "Section".

On peut la faire devenir un form soit :

  • avec un role="form". VoiceOver ne dira rien de plus.
  • avec un title="xxx", VoiceOver annonce "xxx, group".
  • avec un aria-label="xxx", VoiceOver annonce "xxx, group".
  • avec un aria-labelledby="xxx", VoiceOver annonce "xxx, group".

Ces 4 méthodes permettent d'utiliser le getByRole('form') de testing-library.

import { hbs } from 'ember-cli-htmlbars';
import { module, test } from 'qunit';
import sinon from 'sinon';
Expand All @@ -25,11 +26,14 @@ module('Integration | Component | Module | QCM', function (hooks) {

// then
assert.ok(screen);
assert.strictEqual(findAll('.element-qcm-header__instruction').length, 1);
assert.strictEqual(findAll('.element-qcm-header__direction').length, 1);
assert.ok(screen.getByText('Instruction'));
assert.ok(screen.getByRole('group', { legend: this.intl.t('pages.modulix.qcm.direction') }));

// Pas possible de faire un `getByRole('form')`. Voir https://github.com/1024pix/pix/pull/8835#discussion_r1596407648
const form = find('form');
assert.dom(form).exists();
const formDescription = find(`#${form.getAttribute('aria-describedby')}`);
dlahaye marked this conversation as resolved.
Show resolved Hide resolved
assert.dom(formDescription).hasText('Instruction');

assert.strictEqual(screen.getAllByRole('checkbox').length, qcmElement.proposals.length);
assert.ok(screen.getByLabelText('checkbox1'));
assert.ok(screen.getByLabelText('checkbox2'));
Expand Down
12 changes: 8 additions & 4 deletions mon-pix/tests/integration/components/module/qcu_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { render } from '@1024pix/ember-testing-library';
import { click, findAll } from '@ember/test-helpers';
// eslint-disable-next-line no-restricted-imports
import { click, find } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { module, test } from 'qunit';
import sinon from 'sinon';
Expand Down Expand Up @@ -27,11 +28,14 @@ module('Integration | Component | Module | QCU', function (hooks) {

// then
assert.ok(screen);
assert.strictEqual(findAll('.element-qcu-header__instruction').length, 1);
assert.strictEqual(findAll('.element-qcu-header__direction').length, 1);
assert.ok(screen.getByText('Instruction'));
assert.ok(screen.getByRole('group', { legend: this.intl.t('pages.modulix.qcu.direction') }));

// Pas possible de faire un `getByRole('form')`. Voir https://github.com/1024pix/pix/pull/8835#discussion_r1596407648
const form = find('form');
assert.dom(form).exists();
const formDescription = find(`#${form.getAttribute('aria-describedby')}`);
assert.dom(formDescription).hasText('Instruction');

assert.strictEqual(screen.getAllByRole('radio').length, qcuElement.proposals.length);
assert.ok(screen.getByLabelText('radio1'));
assert.ok(screen.getByLabelText('radio2'));
Expand Down
24 changes: 13 additions & 11 deletions mon-pix/tests/integration/components/module/qrocm_test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { clickByName, render } from '@1024pix/ember-testing-library';
import { click, fillIn } from '@ember/test-helpers';
// eslint-disable-next-line no-restricted-imports
import { click, fillIn, find } from '@ember/test-helpers';
import { hbs } from 'ember-cli-htmlbars';
import { module, test } from 'qunit';
import sinon from 'sinon';
Expand Down Expand Up @@ -48,19 +49,20 @@ module('Integration | Component | Module | QROCM', function (hooks) {
type: 'qrocm',
};
this.set('el', qrocm);
const screen = await render(hbs`
<Module::Qrocm @element={{this.el}} />`);
const screen = await render(hbs`<Module::Qrocm @element={{this.el}} />`);
const direction = this.intl.t('pages.modulix.qrocm.direction', {
count: qrocm.proposals.filter(({ type }) => type !== 'text').length,
});

// then
assert.ok(screen);
assert.dom(screen.getByText('Mon instruction')).exists({ count: 1 });
assert.ok(
screen.getByRole('group', {
legend: this.intl.t('pages.modulix.qrocm.direction', {
count: qrocm.proposals.filter(({ type }) => type !== 'text').length,
}),
}),
);
assert.ok(screen.getByRole('group', { legend: direction }));

const form = find('form');
assert.dom(form).exists();
const formDescription = find(`#${form.getAttribute('aria-describedby')}`);
assert.dom(formDescription).hasText('Mon instruction');

assert.dom(screen.getByText('Ma première proposition')).exists({ count: 1 });
assert.ok(screen.getByRole('textbox', { name: 'input-aria' }));
assert.dom(screen.getByText("l'identifiant")).exists({ count: 1 });
Expand Down