Skip to content

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

Open
kayjoosten wants to merge 1 commit into
wayf-reset-endpointfrom
wayf-remember-choice-checkbox
Open

kayjoosten wants to merge 1 commit into
wayf-reset-endpointfrom
wayf-remember-choice-checkbox

Conversation

@kayjoosten

Copy link
Copy Markdown
Contributor

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

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant