Redesign remember-my-choice checkbox in WAYF by kayjoosten · Pull Request #2095 · OpenConext/OpenConext-engineblock

kayjoosten · 2026-09-15T11:20:05Z

What

Implements #2066, redesigning 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 adding an accessible tooltip
on the per-SP variant explaining how long a choice is remembered and
how to reset it.

Stacked on #2094

This branches from wayf-reset-endpoint (#2094, not yet merged), since
the tooltip copy links to the reset endpoint added there, and the
per-SP flag/config this ticket builds on comes from #2065 (#2091). The
diff shown here is the incremental change for #2066 only; once #2091
and #2094 merge to main, this PR's base should be retargeted to
main.

Details

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
pattern 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 and
handles the aria-pressed/aria-expanded toggling generically.
rememberChoicePerIdp and rememberChoiceDuration were already
threaded through WayfViewModel/WayfViewModelFactory/WayfRenderer
for Remember choice for WAYF per SP #2065 but never reached the template — a real pre-existing gap.
wayf.html.twig and remainingIdps.html.twig now forward both
values into rememberChoice.html.twig.
New RememberChoiceDurationFormatter turns
wayf.remember_choice_per_idp_lifetime (seconds) into a translated
"N days"/"N minutes" string. WayfViewModelFactory only computes it
when rememberChoicePerIdp is true, so the translator isn't invoked
on every WAYF render for SPs using the global (non-per-SP) cookie.
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 for all current callers) so both variants can be
exercised manually and from Cypress.
New Cypress coverage in wayf.general.spec.js checks that the per-SP
checkbox and tooltip toggle render and function correctly, and that
the global variant has no tooltip element.

Note: pre-existing `it.only` in wayf.general.spec.js

This spec file has a pre-existing it.only('Shows the global site notice', ...) (predates this branch) that currently suppresses every
other test in the file when run locally/in CI. I verified this is
unrelated to my changes: temporarily removing it against the
unmodified base branch reproduces the same 12 pre-existing failures
(IdP count/search assertions), and with it removed my 5 new tests all
pass cleanly. I left it in place per the minimal-diff policy, but
flagging it here since it means the new Cypress tests (and the rest of
this file) aren't currently exercised by CI as things stand — worth a
follow-up ticket to fix the underlying flakiness and remove .only.

Testing

All run inside the Docker dev container (PHP 8.5):

phpmd / phpcs / phpcs-legacy / docheader: clean.
eb4: 244/244. unit: 1013/1013 (1003 baseline + 10 new).
functional (APP_ENV=test): 121/121. integration: 105/105.
Behat default suite: 300/300 scenarios, 5557/5557 steps.
Twig lint: 115/115. yarn lint: clean.
Cypress (wayf.general.spec.js, firefox headless, with the
pre-existing it.only temporarily removed for verification only):
new tests pass; the 12 pre-existing failures are unchanged from the
base branch (see note above).

Refs: #2066

# 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.

# 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.

# 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.

# 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.

# 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

johanib · 2026-10-07T09:38:50Z

-        <label for="rememberChoice">{{ 'remember_choice'|trans }}</label>
+    <form method="post" action="{{ action }}" class="wayf__rememberChoice{% if rememberChoicePerIdp %} wayf__rememberChoice--perIdp{% endif %}">
        <input type="checkbox" name="rememberChoice" id="rememberChoice">
+        {% if rememberChoicePerIdp %}


Should this be nested inside the rememberChoiceFeature flag?

I think not, since rememberChoiceFeature is a ui only feature.
While rememberChoicePerIdp skips the whole UI. So why should the UI feature be on.

johanib · 2026-10-07T09:55:51Z

        <input type="checkbox" name="rememberChoice" id="rememberChoice">
+        {% if rememberChoicePerIdp %}
+            <label for="rememberChoice">{{ 'remember_choice_per_idp'|trans({ '%duration%': rememberChoiceDuration }) }}</label>
+            <input type="checkbox" tabindex="-1" class="tooltip visually-hidden" aria-expanded="false" role="button" aria-pressed="false" id="rememberChoiceTooltip" name="rememberChoiceTooltip">


why tabindex -1, while the input above this input does not have a tabindex?

# 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.

# 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.

# 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

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

# 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.

# 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.

# 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

# 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.

kayjoosten force-pushed the wayf-reset-endpoint branch from 3081177 to a362211 Compare September 24, 2026 07:19

kayjoosten force-pushed the wayf-remember-choice-checkbox branch from c80bd7c to 900a703 Compare September 24, 2026 07:20

johanib reviewed Oct 7, 2026

View reviewed changes

kayjoosten force-pushed the wayf-reset-endpoint branch from a362211 to 0f9a300 Compare October 8, 2026 08:27

kayjoosten force-pushed the wayf-remember-choice-checkbox branch from 83839c3 to 2de9496 Compare October 8, 2026 08:27

kayjoosten added 6 commits October 8, 2026 10:46

Remove explanatory class docblock restating what the test renders

14d96b8

kayjoosten force-pushed the wayf-reset-endpoint branch from 0f9a300 to a7aacbc Compare October 8, 2026 08:49

kayjoosten force-pushed the wayf-remember-choice-checkbox branch from 2de9496 to 059e6ca Compare October 8, 2026 08:49

Navigation Menu

Repository navigation

Redesign remember-my-choice checkbox in WAYF - #2095

Redesign remember-my-choice checkbox in WAYF#2095
kayjoosten wants to merge 6 commits into
wayf-reset-endpointfrom
wayf-remember-choice-checkbox

kayjoosten commented Sep 15, 2026

Uh oh!

johanib Oct 7, 2026

Uh oh!

johanib Oct 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

kayjoosten commented Sep 15, 2026

What

Stacked on #2094

Details

Note: pre-existing it.only in wayf.general.spec.js

Testing

Uh oh!

johanib Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

johanib Oct 7, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Note: pre-existing `it.only` in wayf.general.spec.js