Repository navigation
Commit 552692e
authored
Redesign remember-my-choice checkbox in WAYF (#2095)
* Redesign remember-my-choice checkbox in WAYF
Update the WAYF "remember my choice" checkbox to match the new visual
design (checkbox before label, adjusted margins, plain black text
instead of bold blue) and add an accessible tooltip on the per-SP
variant that explains how long a choice is remembered and how to reset
it, using the existing tooltip pattern from consent's attribute list.
- rememberChoice.html.twig is reordered so the checkbox comes before
its label, and now branches on rememberChoicePerIdp: the per-SP
variant shows duration-aware copy ("remembered for 90 days") plus a
tooltip toggle, while the global variant keeps its existing text
unchanged. The tooltip reuses the same checkbox-toggle + CSS sibling
selector approach as Consent/Attributes/tooltip.html.twig, so no new
JavaScript is needed: wayf/mouseBehaviour.js's existing
handleAriaPressed() already runs on every WAYF page load.
- rememberChoicePerIdp and rememberChoiceDuration were already
threaded through WayfViewModel/WayfViewModelFactory/WayfRenderer for
#2065 but never reached the template: wayf.html.twig and
remainingIdps.html.twig now forward both values into
rememberChoice.html.twig.
- RememberChoiceDurationFormatter turns
wayf.remember_choice_per_idp_lifetime (seconds) into a translated
"N days"/"N minutes" string, computed once in WayfViewModelFactory
and only when rememberChoicePerIdp is true, so the translator isn't
invoked on every WAYF render for SPs that don't use the feature.
- New translation keys were added to languages/messages.{en,nl,pt}.php
rather than theme/*/translations, matching where the existing
remember_choice key already lives.
- WayfController's functional-testing fixture gained a
rememberChoicePerIdp query param (defaulting to false, preserving
existing behaviour) so both the per-SP and global variants can be
exercised manually and from Cypress.
- New Cypress coverage in wayf.general.spec.js checks the per-SP
checkbox and tooltip toggle render correctly and that the global
variant has no tooltip element.
Refs: #2066
* Wire WAYF tooltip aria handlers
# If applied, this commit will
Wire the generic tooltip ARIA handlers into the WAYF callback, add a Cypress regression test for the remember-choice tooltip toggle, and restore the spec to run fully by removing a stray it.only.
# Why is this change needed?
Prior to this change, the WAYF remember-choice tooltip never updated its aria-hidden and aria-expanded state when the label was clicked, so assistive technology received incorrect state.
# How does it address the issue?
This change invokes the existing tooltip accessibility handlers during WAYF initialization and adds a regression test that checks the ARIA state changes.
# Provide links to any relevant tickets, articles or other resources
Found during review of PR #2095.
* Handle Enter on WAYF tooltip toggle
# If applied, this commit will
Make the remember-choice tooltip open when its label is focused and Enter is pressed, and add a Cypress regression test for that keyboard interaction.
# Why is this change needed?
Prior to this change, pressing Enter on the focused WAYF tooltip label did nothing because the Enter handler ignored tooltip labels.
# How does it address the issue?
This change routes tooltip labels through the existing click helper in the WAYF Enter handler so the keyboard interaction follows the same path as a mouse click.
# Provide links to any relevant tickets, articles or other resources
Found during review of PR #2095.
* Remove explanatory class docblock restating what the test renders
* Fix tooltip click target and keyboard submit in WAYF remember choice
# If applied, this commit will...
Fix two bugs in the per-IdP remember-choice tooltip introduced by the
tooltip redesign, and restore the test scoping that was accidentally
removed while doing so.
# Why is this change needed?
Prior to this change, the tooltip label had no explicit display value,
so it defaulted to inline and its width/height rules from the tooltip
mixin had no effect, collapsing it to a zero-size element that could
not be clicked. Separately, pressing Enter while the tooltip label was
focused triggered the browser's native implicit submission of the
surrounding WAYF form, navigating away instead of just opening the
tooltip.
# How does it address the issue?
This change sets the tooltip label to display: inline-block so its
sizing rules apply, matching the equivalent consent flow styling, and
calls preventDefault on the Enter keydown for the tooltip case so only
the intended click behaviour runs. It also restores it.only on the
unrelated site notice test, which had been dropped as a side effect of
an earlier change in this branch and unintentionally widened this PR's
test run to unrelated, pre-existing failures out of scope here.
# Provide links to any relevant tickets, articles or other resources
#2095
* Let rememberChoicePerIdp render the mock WAYF checkbox on its own
# Why is this change needed?
Prior to this change, the mock WAYF page only showed the remember my choice
checkbox in per-SP mode when both rememberChoiceFeature and
rememberChoicePerIdp were passed. In production only one mode can be enabled,
so the per-SP mode never has the global flag set. The parameter was also
missing from the testing docs.
# How does it address the issue?
This change makes rememberChoicePerIdp imply the feature, like in production,
and documents both modes and example links in docs/testing.md and
docs/js_testing.md.
* Split the WAYF remember-my-choice template per mode
# If applied, this commit will...
Render the global and the per-SP remember-my-choice checkbox from two
separate templates, selected by a thin if/elseif in rememberChoice.html.twig.
# Why is this change needed?
Prior to this change, one template held both variants and nested the per-SP
checkbox inside the rememberChoiceFeature flag. The two modes are mutually
exclusive, so the per-SP variant should not depend on the flag of the global
variant, and the nesting hid that either/or relationship.
# How does it address the issue?
This change moves each variant into its own partial
(rememberChoiceGlobal and rememberChoicePerIdp). The per-SP variant is
rendered whenever rememberChoicePerIdp is set, the global variant only
otherwise. The markup of both variants is unchanged. The render test now also
covers the per-SP variant without the global flag and the case where both are
disabled.
# Provide links to any relevant tickets, articles or other resources
Review feedback on the remember-my-choice checkbox pull request.
* Fix double aria-pressed flip on the WAYF tooltip toggle
Prior to this change, the per-SP remember-choice tooltip checkbox had
aria-pressed flipped twice per label click on the WAYF. mouseBehaviour()
attached changeAriaPressed to every input[aria-pressed], and
toggleTooltipPressedStates() flips it as well. The attribute therefore
ended at its original value while aria-expanded changed, giving screen
readers a wrong pressed state.
This change skips tooltip toggles in the generic aria-pressed handler of
mouseBehaviour(), leaving toggleTooltipPressedStates() as the only owner,
as on the consent page. The per-SP tooltip tests move to their own spec
so they run regardless of the it.only in wayf.general.spec.js, and now
assert aria-pressed in both states.1 parent b6d8cad commit 552692e
25 files changed
Lines changed: 459 additions & 18 deletions
File tree
- config/services
- docs
- languages
- src/OpenConext
- EngineBlockBundle
- Service
- ViewModel
- EngineBlockFunctionalTestingBundle/Controllers
- tests
- e2e/cypress/integration/skeune/wayf
- unit/OpenConext/EngineBlockBundle
- Service
- Twig
- theme/base
- javascripts
- wayf
- stylesheets/pages/wayf
- templates/modules/Authentication/View/Proxy
- Partials/WAYF
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
4 | 4 | | |
5 | 5 | | |
6 | 6 | | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
7 | 12 | | |
8 | 13 | | |
9 | 14 | | |
| 15 | + | |
10 | 16 | | |
11 | 17 | | |
12 | 18 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
42 | 42 | | |
43 | 43 | | |
44 | 44 | | |
45 | | - | |
| 45 | + | |
| 46 | + | |
46 | 47 | | |
47 | 48 | | |
48 | 49 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
20 | 20 | | |
21 | 21 | | |
22 | 22 | | |
23 | | - | |
| 23 | + | |
| 24 | + | |
24 | 25 | | |
25 | 26 | | |
26 | 27 | | |
| |||
38 | 39 | | |
39 | 40 | | |
40 | 41 | | |
41 | | - | |
| 42 | + | |
| 43 | + | |
42 | 44 | | |
43 | 45 | | |
44 | 46 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
86 | 86 | | |
87 | 87 | | |
88 | 88 | | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
89 | 94 | | |
90 | 95 | | |
91 | 96 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
85 | 85 | | |
86 | 86 | | |
87 | 87 | | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
88 | 93 | | |
89 | 94 | | |
90 | 95 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
82 | 82 | | |
83 | 83 | | |
84 | 84 | | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
85 | 90 | | |
86 | 91 | | |
87 | 92 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
| 31 | + | |
31 | 32 | | |
32 | 33 | | |
33 | 34 | | |
| |||
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
| 74 | + | |
73 | 75 | | |
74 | 76 | | |
75 | 77 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
53 | 53 | | |
54 | 54 | | |
55 | 55 | | |
| 56 | + | |
56 | 57 | | |
57 | 58 | | |
58 | 59 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
44 | 44 | | |
45 | 45 | | |
46 | 46 | | |
| 47 | + | |
47 | 48 | | |
48 | 49 | | |
49 | 50 | | |
| |||
66 | 67 | | |
67 | 68 | | |
68 | 69 | | |
69 | | - | |
| 70 | + | |
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
| 74 | + | |
73 | 75 | | |
74 | 76 | | |
75 | 77 | | |
| |||
0 commit comments