Skip to content

Fix tabbing between fields not working with mixed_text field and add style for disabled 'Next' button. - #1685

Open
rute-santos wants to merge 1 commit into
opencast:r/20.xfrom
harvard-dce:fix_mixed_text
Open

rute-santos wants to merge 1 commit into
opencast:r/20.xfrom
harvard-dce:fix_mixed_text

Conversation

@rute-santos

Copy link
Copy Markdown
Contributor

Fix keyboard navigation through mixed_text metadata fields; style disabled wizard buttons

Problem

In the wizards (e.g. "Add event"), multi-value mixed_text fields such as
Presenter(s) or Contributor(s) broke keyboard navigation:

  • Pressing Tab out of the field left it in edit mode, with the typed text
    held only in local state. The value was never committed, so required fields
    stayed invalid and the Next button stayed disabled with no visible
    reason.
  • Tab also stopped on every value's remove ("x") button before reaching the
    next field.
  • Sometimes focus was lost entirely after tabbing out, so the caret went
    nowhere.
  • All RenderMultiField instances shared one module-level childRef, so the
    click-outside detection was attached to whichever field rendered last.

Separately, disabled Next/Create buttons looked the same as enabled
ones. NavigationButtons already sets inactive/disabled on them, but
nothing styled those states.

Changes

  • RenderMultiField.tsx
    • childRef is now created per component with useRef. It is passed to
      EditMultiSelect as containerRef.
    • A new onBlur on the field container checks relatedTarget. When focus
      leaves the field, the typed value is committed and the field leaves edit
      mode. When focus moves inside the field, the value is committed and the
      field stays open. The handler runs on the next tick so it doesn't
      re-render while the browser is still moving focus. Clicks are still
      handled by useClickOutsideField.
    • submitValue is called through a ref, so callers that run outside the
      current render (the unmount cleanup) use the latest field value. Before,
      they could overwrite values added in the meantime.
    • The remove buttons have tabIndex={-1}, like the remove links in the
      legacy admin UI. Values can still be removed from the keyboard: pressing
      Backspace in the empty input removes the last value.
  • wizardHooks.ts: adds a comment explaining why useClickOutsideField
    doesn't handle keyboard exit itself.
  • _footer.scss: wizard footer buttons with .inactive or :disabled
    are shown at 50% opacity with a default cursor.

How to test

  1. Open Add event and go to the metadata step.
  2. Click into Presenter(s), type a name, and press Tab. The name should
    become a label, the field should close, and focus should land on the next
    field.
  3. Tab through a field that has several values. Focus should not stop on the
    "x" buttons.
  4. In an open multi-value field with an empty input, press Backspace. The
    last value should be removed.
  5. Click inside an open field (input, datalist, existing labels). It should
    stay open. Click outside it. It should close and keep the typed value.
  6. Leave a required field empty. Next should look greyed out, and it
    should look normal again once the field is filled.

AI Usage

🤖 Generated with Claude Code

…tyle for disabled 'Next' button.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@rute-santos rute-santos changed the title Fixed tabbing between fields not working with mixed_text field. Add s… Fix tabbing between fields not working with mixed_text field and add style for disabled 'Next' button. Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

This pull request is deployed at test.admin-interface.opencast.org/1685/2026-10-02_20-00-38/ .
It might take a few minutes for it to become available.

@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

Use docker or podman to test this pull request locally.

Run test server using develop.opencast.org as backend:

podman run --rm -it -p 127.0.0.1:3000:3000 

Specify a different backend like stable.opencast.org:

podman run --rm -it -p 127.0.0.1:3000:3000 -e PROXY_TARGET=https://stable.opencast.org 

It may take a few seconds for the interface to spin up.
It will then be available at http://127.0.0.1:3000.
For more options you can pass on to the proxy, take a look at the README.md.

@Arnei

Arnei commented Oct 5, 2026 •

Copy link
Copy Markdown
Member

The remove buttons have tabIndex={-1}, like the remove links in the legacy admin UI. Values can still be removed from the keyboard: pressing Backspace in the empty input removes the last value.

I am against this part. Having important control elements be inaccessible to keyboard controls hurts accessibility. And the proposed alternative is hardly adequate. It seems very unintuitive and poses usability concerns (Want to delete the first element in the list? Welp, you'll have to delete all other elements first).

Comment on lines 167 to +168
handleBlur: (refCurrent: string) => void
handleLeave: (typedValue: string) => void

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd like better naming for these two handlers, to make it more clear which cases they are supposed to cover. Anyone reading this at a glance would likely be confused because they'd assume "blur" and "leave" to be the same thing.

const typedValue = inputValue;

// Defer until the browser has finished moving focus; re-rendering
// mid-transfer would otherwise lose focus entirely.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this an actual issue, or is Claude Code hallucinating problems into existence that aren't there? The code could be a lot simpler if this is not an actual issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I suspect you are right...

Comment on lines +52 to +58
// Grey out disabled wizard buttons. Must stay after the btn() includes
// to override their :hover/:focus rules.
&.inactive,
&:disabled {
opacity: 0.5;
cursor: default;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Completely unrelated to the main issue this PR solves, should at least be its own commit

This branch has not been deployed

No deployments
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.

2 participants