-
Notifications
You must be signed in to change notification settings - Fork 0
Add length constraint fields to Text property editor #470
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
Conversation
Add minLength and maxLength fields to TextAttributesEditor with natural language UI
("between X and Y characters") and validation:
- Positive integers >= 1 only
- minLength cannot exceed maxLength
- Invalid values are not emitted to parent
Also fixes Text.createPropertyDefinitionFromJson to include minLength/maxLength.
Manually tested in browser. All code changes thoroughly reviewed.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Deliberate deviation from UI approach in the numberattributeseditor. To try out if this works better. Original one for numbers:
Now we have essentially the same attributes for text field length, but different UI approach:
This shows they logically go together, esp nice for when validation fails with the min being higher than the max. @alistair3149 @malberts what do you think of this approach? We could use the same one for numberattributeseditor min and max. I did a bunch of refining already. Known thing not optimized: small screen sizes. |
Do you mean two rows, like we have for min and max of number properties? If not, can you link or screenshot an example? We might allow more complex constraints via another mechanism at a later point, no need to care about this now. |
|
Observation: there might not be a single approach without a list of downsides. We also do not need to find the perfect one. So, we identified a bunch of downsides for the approach in this PR. Are there approaches that have a more appealing set of tradeoffs? In particular, the original approach, which we have still in use at the number type, does not have a clear spot to show the "min should be lower than max" error, nor does it visually group these fields in any way to suggest there is a relation between them. This might still be overall better than the approach in this PR. Is that what you think? |
|
Looks good to me If the rest of the code looks fine, we could merge this and follow up with moving the UI elements |
|
The code looks good to me, we can follow up with the UI later like @JeroenDeDauw mentioned. |







Add minLength and maxLength fields to TextAttributesEditor with natural language UI
("between X and Y characters") and validation:
Also fixes Text.createPropertyDefinitionFromJson to include minLength/maxLength.
Manually tested in browser. All code changes thoroughly reviewed by Jeroen and Claude
Co-Authored-By: Claude Opus 4.5 noreply@anthropic.com