fix(dialog): drop the toggle from free text properties - #275
Merged
Merged
Conversation
The bars were positioned by the panel's own bottom padding, which left them floating above the content when the form was shorter than the dialog. Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
A note has an obvious empty state, so the switch beside it only added a step: the field was disabled until it was flipped on, and flipping it off was the way to remove a note. Render free text without a toggle instead, always editable, and let an empty field mean the property is unset — which is what a required note already did. Clearing the text is now the only way to unset such a property, so a pending edit is flushed when the field goes away; the debounce would otherwise keep the old note when the dialog closes right after typing. Properties that are not free text keep their toggle: for a password or an expiry, "on with no value yet" is a state worth expressing, and switching one off says something the empty value alone does not. Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #275 +/- ##
==========================================
+ Coverage 79.06% 79.91% +0.85%
==========================================
Files 27 27
Lines 726 727 +1
Branches 202 202
==========================================
+ Hits 574 581 +7
+ Misses 133 127 -6
Partials 19 19 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
1 participant
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.
The note field had a switch beside it: the textarea stayed disabled until the switch was flipped on, and flipping it off was how a note got removed. An empty text field already says "no note", so free text now renders without a toggle, always editable, and an empty one means the property is unset — which is what a required note already did.
The rule is
isLongTextProperty, not the note's own class. A password or an expiry keeps its toggle, because "on with no value yet" is a state worth expressing there and switching one off says something an empty value does not.Clearing the text is now the only way to unset such a property, so a pending edit is flushed when the field unmounts; the 500 ms debounce would otherwise keep the old note when the dialog closes right after typing.
Also sticks the action bars to the bottom of the panel — they were positioned by the panel's own bottom padding and floated above short forms.
Four new tests cover free text rendering without a toggle, a password keeping one, clearing a field dispatching
setProperty(class, null), and a pending edit surviving unmount.👾 This pull request was assisted by Claude Code, commits carry an
Assisted-bytrailer.