Skip to content

Conversation

MrHutmat
Copy link
Contributor

Added custom validation for missing password and user/email

Added support for the message to use Localization

Moved the logic that checks for empty username and password from login.page.element.ts to auth.element.ts

login.example.mp4

@Copilot Copilot AI review requested due to automatic review settings September 23, 2025 11:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR enhances the login form validation by adding custom client-side validation messages for empty username/email and password fields with localization support.

  • Moved validation logic from login page to the auth element for better separation of concerns
  • Added localized validation messages for required field validation
  • Implemented custom CSS styling for validation message display

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/Umbraco.Web.UI.Login/src/localization/lang/en-us.ts Added English localization keys for validation messages
src/Umbraco.Web.UI.Login/src/localization/lang/da.ts Added Danish localization keys for validation messages
src/Umbraco.Web.UI.Login/src/components/pages/login.page.element.ts Removed validation logic and cleaned up error state handling
src/Umbraco.Web.UI.Login/src/auth.element.ts Added validation logic with custom validation messages and form submission handling
src/Umbraco.Web.UI.Login/src/auth-styles.css Added CSS styling for validation messages and minor formatting fix

Comment on lines 83 to 110
form.addEventListener('submit', (e) => {
e.preventDefault();

const form = e.target as HTMLFormElement;
if (!form) return;

const formData = new FormData(form);

const username = formData.get('username') as string;
const password = formData.get('password') as string;

const validationMessages = Array.from(form.querySelectorAll('.validation-message'));
validationMessages.forEach((vm) => vm.removeAttribute('state'));

if (!username) {
const validationMessage = validationMessages.find(
(vm) => vm.getAttribute('for') === 'username-input'
) as UUIFormValidationMessageElement;
validationMessage?.setAttribute('state', 'error');
}

if (!password) {
const validationMessage = validationMessages.find(
(vm) => vm.getAttribute('for') === 'password-input'
) as UUIFormValidationMessageElement;
validationMessage?.setAttribute('state', 'error');
}
});
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

The form submission validation blocks all form submissions when fields are empty, but doesn't allow valid submissions to proceed to the original form handler. The validation should only show errors for empty fields but still allow the form to be submitted when validation passes.

Copilot uses AI. Check for mistakes.

Comment on lines 52 to 60
const createValidationMessage = (id: string, localizationKey: string) => {
const validationElement = document.createElement('div');
validationElement.className = 'validation-message';
validationElement.setAttribute('for', id);
const localizeElement = document.createElement('umb-localize');
localizeElement.key = localizationKey;
validationElement.appendChild(localizeElement);
return validationElement;
};
Copy link

Copilot AI Sep 23, 2025

Choose a reason for hiding this comment

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

[nitpick] The validation message container uses a generic div element. Consider using semantic HTML elements like <span role='alert'> or the proper UUI validation message component for better accessibility and consistency with the UI framework.

Copilot uses AI. Check for mistakes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants