Skip to content

web: Form validation regressions, consistency fixes #15894

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 16 commits into from
Aug 12, 2025

Conversation

GirlBossRush
Copy link
Contributor

@GirlBossRush GirlBossRush commented Jul 30, 2025

Details

This PR resolves a collection of form validation issues, as well as a partial fix for our WebDriverIO tests to run while Playwright is in review.

Fixed Issues

  • ErrorProps containing strings are now parsed correctly instead of defaulting to a generic network error
  • Single-step modal forms perform client-side validation before submitting their data
  • Wizards now perform client-side validation before progressing to the next step
  • Form Groups no longer fight for focus when multiple invalid inputs exist.
  • Form Groups no longer scroll into view when a previously invalid field becomes valid
  • Invalid inputs within Form Groups now wait for their parent to expand before attempting to focus the element while still hidden
  • Both server and client-side errors now trigger Form Group expansion.
  • Inputs with server-side errors are now marked with ARIA attributes to match their status
  • The inner form element getter within AKForm now uses renderRoot over shadowRoot, fixing selector issues when in compatibility mode.

See also

@GirlBossRush GirlBossRush requested a review from a team as a code owner July 30, 2025 23:36
Copy link

netlify bot commented Jul 30, 2025

Deploy Preview for authentik-docs canceled.

Name Link
🔨 Latest commit 31db063
🔍 Latest deploy log https://app.netlify.com/projects/authentik-docs/deploys/689b48fee219de0008da8efb

Copy link

netlify bot commented Jul 30, 2025

Deploy Preview for authentik-integrations canceled.

Name Link
🔨 Latest commit 31db063
🔍 Latest deploy log https://app.netlify.com/projects/authentik-integrations/deploys/689b48fe03c66300085e39c7

Copy link

netlify bot commented Jul 30, 2025

Deploy Preview for authentik-storybook ready!

Name Link
🔨 Latest commit 31db063
🔍 Latest deploy log https://app.netlify.com/projects/authentik-storybook/deploys/689b48fe94755f0008a7b558
😎 Deploy Preview https://deploy-preview-15894--authentik-storybook.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

codecov bot commented Jul 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.76%. Comparing base (e154fee) to head (31db063).
⚠️ Report is 8 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #15894      +/-   ##
==========================================
- Coverage   92.83%   92.76%   -0.07%     
==========================================
  Files         836      836              
  Lines       45160    45166       +6     
==========================================
- Hits        41924    41900      -24     
- Misses       3236     3266      +30     
Flag Coverage Δ
e2e 46.60% <ø> (-0.09%) ⬇️
integration 23.59% <ø> (+0.01%) ⬆️
unit 90.91% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jul 30, 2025

authentik PR Installation instructions

Instructions for docker-compose

Add the following block to your .env file:

AUTHENTIK_IMAGE=ghcr.io/goauthentik/dev-server
AUTHENTIK_TAG=gh-31db063e479c1c0ed00fef1f8d9dd22dc8808117
AUTHENTIK_OUTPOSTS__CONTAINER_IMAGE_BASE=ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s

Afterwards, run the upgrade commands from the latest release notes.

Instructions for Kubernetes

Add the following block to your values.yml file:

authentik:
    outposts:
        container_image_base: ghcr.io/goauthentik/dev-%(type)s:gh-%(build_hash)s
global:
    image:
        repository: ghcr.io/goauthentik/dev-server
        tag: gh-31db063e479c1c0ed00fef1f8d9dd22dc8808117

Afterwards, run the upgrade commands from the latest release notes.

@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch 4 times, most recently from 9f9e667 to 27a772a Compare August 3, 2025 17:33
@GirlBossRush GirlBossRush mentioned this pull request Aug 3, 2025
6 tasks
@GirlBossRush GirlBossRush changed the base branch from main to a11y-form-errors August 3, 2025 17:49
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch from 6cd91f9 to fa9e7fc Compare August 3, 2025 18:09
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch from fa9e7fc to bb636e7 Compare August 4, 2025 09:56
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch 2 times, most recently from 921644b to 9ca6463 Compare August 4, 2025 19:19
@GirlBossRush GirlBossRush requested a review from a team as a code owner August 4, 2025 19:19
@GirlBossRush GirlBossRush changed the base branch from a11y-form-errors to main August 4, 2025 19:21
@GirlBossRush GirlBossRush removed the request for review from a team August 4, 2025 19:21
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch 4 times, most recently from 50deedb to 7ab6db0 Compare August 6, 2025 17:13
@GirlBossRush GirlBossRush added the a11y Features or bugs related to accessibility label Aug 12, 2025
@GirlBossRush GirlBossRush changed the title web/a11y: Labels, Functional components. web/a11y: Form validation Aug 12, 2025
@GirlBossRush GirlBossRush changed the title web/a11y: Form validation web: Fix form validation regressions, expand coverage of validation Aug 12, 2025
@GirlBossRush GirlBossRush changed the title web: Fix form validation regressions, expand coverage of validation web: Form validation regressions, consistency fixes Aug 12, 2025
Comment on lines -196 to +197
"@web/test-runner": "^0.20.2"
"@web/test-runner": "^0.20.2",
"chromedriver": "^139.0.0"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

WebDriver is still plagued with download issues, but this inclusion will let the legacy e2e test suite run when Chromium is manually installed.

}

/**
* @todo This defaults to true when the form is not yet available
Copy link
Contributor Author

@GirlBossRush GirlBossRush Aug 12, 2025

Choose a reason for hiding this comment

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

TOOD: Circle back on this after #16010 gives us a reusable pattern with form internals

Comment on lines +66 to +68
if (!this.form) {
throw new TypeError("Form reference is not set");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unlikely but this has been technically possible with all uses of @query

Comment on lines +46 to +48
public get form(): HTMLFormElement | null {
return this.renderRoot.querySelector("form#applicationform");
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Switch to getter to allow for overriding in application provider steps

@@ -71,7 +72,7 @@ export class ApplicationWizardApplicationStep extends ApplicationWizardStep {
this.errors.set("name", msg("An application name is required"));
}

if (!values.metaLaunchUrl || !URL.canParse(values.metaLaunchUrl)) {
if (values.metaLaunchUrl && !URL.canParse(values.metaLaunchUrl)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TODO: This was subtle fix. Circle back on something like an <ak-url-input> to handle this sort of check.

const providerForm = this.element.form;

if (!providerForm) {
// TODO: This needs to be removed once all steps can report their validity.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAICT this is only true of the bindings step which has no form. Leaving it in for now to reduce the number of changes.

@@ -16,7 +16,7 @@ import { snakeCase } from "change-case";
import { CSSResult } from "lit";
import { property, query } from "lit/decorators.js";

export class ApplicationWizardProviderForm<T extends OneOfProvider> extends AKElement {
export abstract class ApplicationWizardProviderForm<T extends OneOfProvider> extends AKElement {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ApplicationWizardStep should also be abstract, but our typing on CustomEmitterElement makes this difficult to achieve without a lot of detangling.

errors,
errors.provider ?? {},
Copy link
Contributor Author

@GirlBossRush GirlBossRush Aug 12, 2025

Choose a reason for hiding this comment

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

AFAICT not showing field errors in the provider step seems to be a long standing bug we managed to surface when error parsing was introduced.

I suspect that ak-application-wizard-submit-step also has an expectation of field error name spacing that doesn't apply anymore.

Comment on lines 38 to 62
.pf-c-form__group {
--pf-c-form--m-horizontal__group-label--md--GridColumnWidth: minmax(max-content, 9.375rem);
column-gap: var(--pf-global--spacer--md);
}

.pf-c-form__group-label {
display: flex;
user-select: none;
padding-top: var(--pf-c-form--m-horizontal__group-label--md--PaddingTop);

/* Increase the pressable area of the label. */
.pf-c-form__label {
display: block;
flex: 1 1 auto;
}
}

.pf-c-form__label[aria-required] .pf-c-form__label-text::after {
content: "*";
user-select: none;
margin-left: var(--pf-c-form__label-required--MarginLeft);
font-size: var(--pf-c-form__label-required--FontSize);
color: var(--pf-c-form__label-required--Color);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Largely moved out of horizontal form for better consistency in inputs across all interfaces.

required?: boolean;
}

export const AKLabel: LitFC<FormLabelProps> = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

See #16119 for how this works with label input association.

@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch from 37a52d6 to 144d377 Compare August 12, 2025 01:09
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
@GirlBossRush GirlBossRush enabled auto-merge (squash) August 12, 2025 13:04
@GirlBossRush GirlBossRush disabled auto-merge August 12, 2025 13:06
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch from 1acb244 to 9a37950 Compare August 12, 2025 13:27
… editing existing entities.

- We need to make the component have a better understanding of this concept.
@GirlBossRush GirlBossRush force-pushed the a11y-functional-label-components branch from 9a37950 to d01b0b9 Compare August 12, 2025 13:48
@GirlBossRush GirlBossRush enabled auto-merge (squash) August 12, 2025 14:00
@GirlBossRush GirlBossRush merged commit 1bc2daa into main Aug 12, 2025
107 checks passed
@GirlBossRush GirlBossRush deleted the a11y-functional-label-components branch August 12, 2025 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a11y Features or bugs related to accessibility bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants