Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an explicit clear (“X”) affordance for TIME property inputs (to compensate for Chrome lacking a native clear UI for <input type="time">) while reusing the existing debounced save/coercion logic so clearing persists null rather than an empty string.
Changes:
- Expose a new
handleInputClear()handler fromusePropertyto reset local input/error state and invoke the existing debounced save path. - Render a TIME-only trailing clear button in
Property.tsxwhen a time value is present. - Add/extend tests to cover the trailingAction click pattern and document the TIME-clear behavior.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
client/src/pages/platform/workflow-editor/components/properties/hooks/useProperty.ts |
Adds handleInputClear to reset value/error state and trigger debounced persistence (empty → null coercion preserved). |
client/src/pages/platform/workflow-editor/components/properties/hooks/tests/timeFieldClear.test.ts |
Adds a new test file to codify TIME-clear expectations (currently duplicates logic / doesn’t verify debounce semantics). |
client/src/pages/platform/workflow-editor/components/properties/components/property-input/PropertyInput.test.tsx |
Adds a unit test ensuring trailingAction click handlers fire (supports TIME clear button pattern). |
client/src/pages/platform/workflow-editor/components/properties/Property.tsx |
Renders a trailing clear button for TIME inputs with a non-empty value, wired to handleInputClear. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Chrome's <input type="time"> does not provide a native clear button, so users had no way to clear a Time field. Expose an explicit X via the existing trailingAction slot that reuses the debounced save path so the empty -> null coercion already in useProperty stays the single source of truth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The null-coercion for DATE/DATE_TIME/TIME is already covered in saveInputValueResolvedValue.test.ts, so re-declaring it here risks drift from useProperty.ts. Also rename the remaining assertion to reflect what it actually checks (save callback invocation, not debounce semantics). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ediately
Calling inputRef.focus() synchronously after setInputValue('') flipped
PropertyInput's isFocused to true in the same React batch, so the
value-sync effect skipped and the stale time remained visible until the
next blur. Schedule focus() via requestAnimationFrame so the empty value
commits while isFocused is still false.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Chrome's does not provide a native clear button,
so users had no way to clear a Time field. Expose an explicit X via the
existing trailingAction slot that reuses the debounced save path so the
empty -> null coercion already in useProperty stays the single source of
truth.
Co-Authored-By: Claude Opus 4.7 (1M context) noreply@anthropic.com