Skip to content

fix(a11y): synchronize editor validation before save activation - #594

Closed
seonghobae wants to merge 1 commit into
developfrom
fix/editor-validation-accessibility-411
Closed

fix(a11y): synchronize editor validation before save activation#594
seonghobae wants to merge 1 commit into
developfrom
fix/editor-validation-accessibility-411

Conversation

@seonghobae

Copy link
Copy Markdown
Contributor

RED phase

This draft starts from protected develop@2c328875e00e86537df3e965170be80532571cad and currently contains only the realistic browser regression for #411 at exact contributor head 1fb8923b675e05bc9ef123fdea93763141bf41cf.

Protected app.js currently drives the editor save state with native disabled from debounced validation. That removes the invalid save control from keyboard focus and permits the browser's native constraint-validation path to short-circuit the application's live validation before the delegated submit handler. The same debounce can also leave a just-corrected draft looking disabled long enough to reject immediate pointer/Enter activation.

The new Playwright acceptance test is intentionally RED until production behavior is repaired. It requires:

  • an invalid save control stays focusable, uses aria-disabled="true", and is programmatically described by editor-errors;
  • invalid activation refreshes the latest custom validation, keeps the editor/save focus, and persists nothing;
  • a just-corrected required field can be saved immediately by pointer activation or Enter without waiting for the debounce; and
  • a just-invalidated draft cannot persist through a stale enabled presentation.

The smallest intended production repair is to keep validateDraft() as the synchronous authority at submit time, prevent native form constraint validation from bypassing that path, and represent the rendered save state with aria-disabled plus aria-describedby instead of native disabled. A stale aria-disabled presentation must not itself become the submit decision.

Do not merge this test-only head. RED evidence, the minimal production repair, focused/full GREEN evidence, current-head review, and all live governance gates are still required.

Refs #411

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 28369090-9a5b-4da9-8ff0-c6a061f6c4b3

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

Closing this draft as superseded by the existing #411 owner lane, PR #468, after a fresh semantic and execution-path comparison.

#594 contains only tests/e2e/editor-validation-accessibility.spec.js; its current package.json does not register that file in test:e2e:cloud, so the successful Server Tests run on exact head 1fb8923b675e05bc9ef123fdea93763141bf41cf never executed the purported RED regression. That success is therefore not evidence for this acceptance path.

PR #468 now carries the same required behavior in the already-registered tests/e2e/editor-validation-sync.spec.js: real pointer and keyboard invalid activations must preserve Save focus, keep the editor open, expose editor-errors, and persist nothing; immediate valid click/Enter still save. Its test-first commit is fca25c7700441acbf1598d26e4eff328c3195ac1, followed by the narrow root-cause repair 815af8138df2454cea99ba1a4a384de4f55d6199, which disables native form constraint interception (form.noValidate = true) while retaining required-field semantics/ARIA so validateDraft() and saveEditor() remain the synchronous persistence gate.

The #594 test has no remaining unique product or acceptance behavior that is not represented more realistically and on an executable CI path in #468. Keeping both branches would duplicate issue #411 ownership and preserve a vacuous test lane. No review threads exist here to resolve.

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