Skip to content

Conversation

@AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented Jun 28, 2025

Description

  • Show error message 500ms after user has stopped typing
  • Apply onInput validation on email inputs
  • Remove ghost div when fix-message-line is true
  • Move space name validator to composable
  • little OcTextInput component clean up

Related Issue

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Types of changes

  • Bugfix
  • Enhancement (a change that doesn't break existing code or deployments)
  • Breaking change (a modification that affects current functionality)
  • Technical debt (addressing code that needs refactoring or improvements)
  • Tests (adding or improving tests)
  • Documentation (updates or additions to documentation)
  • Maintenance (like dependency updates or tooling adjustments)

@AlexAndBear AlexAndBear requested review from Copilot, kulmann and tammi-23 and removed request for Copilot June 28, 2025 15:58
Copy link
Contributor

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 pull request updates the visual representation of file name input errors by replacing the deprecated checkSpaceNameModalInput validation with the new isSpaceNameValid validation and enhancing error message handling across components. Key changes include:

  • Replacing checkSpaceNameModalInput with isSpaceNameValid in various actions and components.
  • Updating tests to remove deprecated validation usage and add coverage for the new validation logic.
  • Enhancing the OcTextInput component to display error messages with a debounced update.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
tests/e2e/support/api/graph/userManagement.ts Changed mutable variable declaration to const.
packages/web-pkg/tests/unit/composables/spaces/useSpaceHelpers.spec.ts Removed tests for checkSpaceNameModalInput.
packages/web-pkg/tests/unit/composables/actions/spaces/useSpaceActionsEditReadmeContent.spec.ts Removed checkSpaceNameModalInput mock.
packages/web-pkg/tests/unit/composables/actions/helpers/useIsResourceNameValid.spec.ts Added tests for new isSpaceNameValid logic.
packages/web-pkg/src/composables/spaces/useSpaceHelpers.ts Removed checkSpaceNameModalInput from the exported API.
packages/web-pkg/src/composables/actions/spaces/useSpaceActionsRename.ts Replaced checkSpaceNameModalInput with an inline validation callback using isSpaceNameValid.
packages/web-pkg/src/composables/actions/helpers/useIsResourceNameValid.ts Added new implementation for isSpaceNameValid.
packages/web-pkg/src/composables/actions/files/useFileActionsCreateSpaceFromResource.ts Updated inline onInput callback to use isSpaceNameValid.
packages/web-pkg/src/components/AppBar/CreateSpace.vue Replaced checkSpaceNameModalInput with isSpaceNameValid in the input callback.
packages/design-system/src/components/OcTextInput/OcTextInput.vue Enhanced error message display with a debounced update via lodash-es.
Comments suppressed due to low confidence (2)

packages/web-pkg/tests/unit/composables/spaces/useSpaceHelpers.spec.ts:4

  • The tests for checkSpaceNameModalInput have been removed. Please ensure that the new isSpaceNameValid validation logic is adequately covered in the unit test suite.
describe('useSpaceHelpers', () => {

packages/design-system/src/components/OcTextInput/OcTextInput.vue:234

  • [nitpick] Introducing a debounced update for error messages can improve UX, but please confirm that the 800ms delay fits the design requirements across different user interactions.
const displayedErrorMessage = ref<string | undefined>(errorMessage)

@AlexAndBear AlexAndBear changed the title improve visual representation on file name input errors feat: improve visual representation on file name input errors Jun 28, 2025
@AlexAndBear
Copy link
Contributor Author

AlexAndBear commented Jun 28, 2025

@kulmann Big Improvement!

Before this change with email validation, we needed to blur (unfocus) the input field, now we validate after 1000ms after the user stopped typing and validate then =)

Copy link
Contributor

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Awesome improvement 😍
One weird thing I found, maybe for a followup: When I create a folder, the error messages from validation are nicely debounced now, but the submit button of the modal still immediately disables/enables based on the validation. 😅 So if I type multiple words as folder name, separated by blanks, each blank will immediately disable the submit button. Does it make sense to debounce the disabled state of the button as well?

@AlexAndBear
Copy link
Contributor Author

AlexAndBear commented Jun 30, 2025

Awesome improvement 😍 One weird thing I found, maybe for a followup: When I create a folder, the error messages from validation are nicely debounced now, but the submit button of the modal still immediately disables/enables based on the validation. 😅 So if I type multiple words as folder name, separated by blanks, each blank will immediately disable the submit button. Does it make sense to debounce the disabled state of the button as well?

I get your point, but to be honest, I would leave everything as it is.
To disable the button immediately is necessary, otherwise we can get in a state were we send unvalidated requests that are prune to errors.
Releasing the button from the disabled state 0.5s after the user stopped typing, feels a little off and blocking.

@AlexAndBear AlexAndBear merged commit 2d042a1 into main Jun 30, 2025
19 checks passed
@AlexAndBear AlexAndBear deleted the issues/870 branch June 30, 2025 14:09
@openclouders openclouders mentioned this pull request Jun 30, 2025
1 task
@openclouders openclouders mentioned this pull request Jul 21, 2025
1 task
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.

File name validation error messages are annoying

3 participants