fix(a11y): Make recurrence modal fully keyboard-accessible

[jensens]

Summary

Fixes plone/Products.CMFPlone#4272

The recurrence modal had two keyboard accessibility issues:

1. "End recurrence" only accessible when tabbing backwards — the modal's focus trap built a static list of focusable elements at open time, missing dynamically shown/hidden fields
2. "Selected dates" and "exclude" buttons unreachable without mouse — occurrence action links loaded via AJAX were never in the focus trap, and used <a href="#"> instead of proper <button> elements

Changes

• Dynamic focus trap (pat/modal/modal.js): activateFocusTrap() now re-queries visible focusable elements on each Tab keypress instead of using a static list captured at init. This benefits all modals across Plone, not just recurrence.
• Semantic buttons (occurrence.xml, display.xml, recurrence.js): Changed occurrence action links and display widget "Add"/"Delete" links from <a href="#"> to <button type="button"> for proper semantics and native keyboard support (Enter + Space).
• CSS updates (recurrence.scss): Updated selector from div.ridisplay .rimain a to div.ridisplay .rimain button. Added :focus-visible outline for occurrence action buttons.
• Tests (recurrence.test.js): Added tests verifying <button> elements are used for display controls and dynamically added date actions.

Test plan

• All 194 existing tests pass (28 suites)
• 2 new tests added and passing
• Manual testing: open an Event edit form, click "Edit" on recurrence, verify Tab reaches all controls including "End recurrence" range options and occurrence exclude/include buttons
• Verify Shift+Tab wraps correctly through the modal
• Verify the "Add"/"Delete rules" buttons in the display widget are keyboard-accessible

[thet]

LGTM

Especially the button change is great.

The dynamic re-evaluation of tab presses sounds a bit expensive, but might be OK too. I'll paste a conversation with an AI on this issue and got a convincing answer, so this improvement LGTM.

[thet]

Btw. I changed the single commit combining two different changes into two commits, as it is the convention in Mockup and Patternslib.

[thet]

thet

Context:

•  modal.js
• agent

@{agent} In the referenced context buffer we have a focusTrapHandler, which is run for each <tab> key press.

1. Does this cover all input focus change possibilities? I mean, are there also other ways of switching the focus from one input to another than pressing the <tab> key, which are not covered by the event handler?

2. Is this the best way to handle dynamic inserted content into a modal, is it performant enought? Or would a MutationObserver be better, which only re-runs the focustraphandler when something has changed?

CodeCompanion (Copilot)

Here are answers to your questions based on the provided code:

1. Does this cover all input focus change possibilities?
   No, the current focus trap handler only intercepts <Tab> key presses within the modal to cycle focus among visible inputs. Other ways users can change focus include:

• Clicking with the mouse (not intercepted by the handler).
• Using <Shift+Tab> (which is covered).
• Using browser/system shortcuts (e.g., accessibility tools, screen readers).
• Programmatic focus changes (e.g., via JavaScript).
• <F6> or <Ctrl+Tab> in some browsers for panel navigation.

So, while the handler covers keyboard navigation via <Tab>, it does not prevent mouse clicks or programmatic focus changes from moving focus outside the modal. For a stricter focus trap, you would need to intercept focus events and redirect them if the target is outside the modal.

2. Is this the best way to handle dynamically inserted content into a modal?
   The current approach re-queries visible inputs on every <Tab> press, ensuring that newly added elements are included in the focus cycle. This is effective and performant for most use cases, as the query is only run when the user presses <Tab>, not on every DOM mutation.

Using a MutationObserver could allow you to update a cached list of focusable elements only when the modal's content changes, potentially improving performance if the modal contains a very large number of elements or if <Tab> is pressed extremely frequently. However, for typical modal content sizes and user behaviour, the difference is negligible, and the current approach is simpler and less error-prone.

Summary:

• The handler is good for keyboard navigation but does not cover all focus change methods.
• For most cases, re-querying on <Tab> is performant and robust. MutationObserver is only needed for very complex/dynamic modals or if you want to cache focusable elements for other reasons.

If you want stricter focus trapping or to experiment with MutationObserver-based caching, let me know and I can implement an example for you.

thet

Context:

•  modal.js
• agent

[jensens]

OK, the AI is right, it covers only tab-key, but that was the original issue: keyboard accessibility. @thet do you merge or shall I?

[petschki]

Just found out that this breaks one robottest here https://github.com/plone/mockup/actions/runs/22964406148/job/66663816140#step:15:107 ... needs to be fixed here https://github.com/plone/plone.app.event/blob/master/src/plone/app/event/tests/robot/test_event_roundtrip.robot#L83

[jensens]

The PR changed <a name="riedit"> to <button type="button" name="riedit">, but the robot test in plone.app.event still looks for //a[@name="riedit"].