-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Implement root property validation for member workspace #19972
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
base: main
Are you sure you want to change the base?
Implement root property validation for member workspace #19972
Conversation
…-view-navigation-context # Conflicts: # src/Umbraco.Web.UI.Client/src/libs/observable-api/states/array-state.ts
# Conflicts: # src/Umbraco.Web.UI.Client/src/packages/core/menu/menu-tree-structure-workspace-context-base.ts
….controller.ts Co-authored-by: Copilot <[email protected]>
…ce/content-validation-to-hints.manager.ts Co-authored-by: Copilot <[email protected]>
…ce/content-validation-to-hints.manager.ts Co-authored-by: Copilot <[email protected]>
There was a problem hiding this 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 implements root property validation for the member workspace, extending previous work to provide real-time validation feedback for member properties like username, email, and password. The implementation introduces a hint system that displays validation issues as badges on workspace view tabs and enables automatic navigation to the problematic fields.
Key changes:
- Adds validation binding for username, email, and password fields in member workspace
- Implements a hint system for displaying validation badges on workspace view tabs
- Creates workspace view contexts and navigation management for better validation UX
- Extends the validation system with value validators for root properties
Reviewed Changes
Copilot reviewed 51 out of 52 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
src/Umbraco.Web.UI.Client/src/packages/members/member/workspace/member/member-workspace.context.ts |
Adds root property validation setup and hint management for member workspace |
src/Umbraco.Web.UI.Client/src/packages/members/member/workspace/member/views/member/member-workspace-view-member.element.ts |
Binds validation to username, email, and password input fields |
src/Umbraco.Web.UI.Client/src/packages/core/workspace/components/workspace-editor/workspace-editor.element.ts |
Implements hint display system for workspace view tabs with badges |
src/Umbraco.Web.UI.Client/src/packages/core/validation/controllers/value-validator/valueValidator.controller.ts |
Creates new value validator controller for handling property-level validation |
src/Umbraco.Web.UI.Client/src/packages/core/hint/ |
Introduces new hint system for managing validation status indicators |
src/Umbraco.Web.UI.Client/src/packages/core/view/ |
Adds view context system for managing workspace view state and hints |
Comments suppressed due to low confidence (1)
src/Umbraco.Web.UI.Client/src/packages/content/content-type/structure/content-type-structure-manager.class.ts:313
- The variable name
missingThis
is unclear and doesn't describe what it represents. Consider renaming to something more descriptive likehasExistingContext
orcontextExists
.
getOwnerContentType() {
return { | ||
path: UMB_WORKSPACE_VIEW_PATH_PATTERN.generateLocal({ viewPathname: manifest.meta.pathname }), | ||
component: () => createExtensionElement(manifest), | ||
setup: (component) => { | ||
setup: (component?: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parameter type any
should be replaced with a more specific type. Consider using Element | undefined
or creating a proper interface for the component.
setup: (component?: any) => { | |
setup: (component?: (Element & { manifest?: unknown }) | undefined) => { |
Copilot uses AI. Check for mistakes.
...Client/src/packages/core/validation/controllers/value-validator/valueValidator.controller.ts
Outdated
Show resolved
Hide resolved
|
||
async validate(): Promise<void> { | ||
this.#validationMode = true; | ||
// Validate is called when the validation state of this validator is asked to be fully resolved. Like when user clicks Save & Publish. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] The comment should end with a period for consistency with documentation standards.
Copilot uses AI. Check for mistakes.
…/feature/implement-validation-hints-for-member-workspace
…lers/value-validator/valueValidator.controller.ts Co-authored-by: Copilot <[email protected]>
@nielslyngsoe there are a few merge conflicts after the base PR has been merged. Would you take a look, please? |
…ember-workspace # Conflicts: # src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/content-validation-to-hints.manager.ts # src/Umbraco.Web.UI.Client/src/packages/content/content/workspace/views/edit/content-editor.element.ts # src/Umbraco.Web.UI.Client/src/packages/core/hint/context/index.ts # src/Umbraco.Web.UI.Client/src/packages/core/view/context/view.context.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works, but eslint is complaining about missing .js endings, and the test suite has encountered a couple of constants that are not exported. When those are fixed, this should be good to go.
Extending the work of #19255 (Should be reviewed first)
By adding a Validator that checks the validity of root properties(username, email, etc.) of the Member Workspace, to then report to the validation system and navigate the user to the info tab.
As well, this uses the Hints Context to display the existence of validation issues on the info Tab.
Fixes #16079