Implements client-side validation of constraint values before you can
add them to a constraint.
I've removed the extra server-side validation that used to happen for
each specific constraint, because the surrounding form itself uses
server side validation to check every constraint every time there's a
change. This is what controls disabling the submit button etc.
I wanna make the next PR a bit of a followup cleanup now that it's
clearer what properties we do and don't need.
<img width="371" alt="image"
src="https://github.com/user-attachments/assets/7c98708f-fcbe-40ca-8590-bb0f5b2ad167"
/>
<img width="361" alt="image"
src="https://github.com/user-attachments/assets/503d4841-d910-4e8e-b0ef-a3d725739534"
/>
Adds inputmode='decimal' to input fields with number input. As discussed
on the [GOV.UK
blog](https://technology.blog.gov.uk/2020/02/24/why-the-gov-uk-design-system-team-changed-the-input-type-for-numbers/),
this finds a balance between giving numeric input options to mobile
devices and improving validation / user experience.
They mention this bit in their [design system
guideline](https://design-system.service.gov.uk/components/text-input/#numbers)
> Do not use `<input type="number">` unless your user research shows
that there’s a need for it. With `<input type="number">` there’s a risk
of users accidentally incrementing a number when they’re trying to do
something else - for example, scroll up or down the page. And if the
user tries to enter something that’s not a number, there’s no explicit
feedback about what they’re doing wrong.
I've purposefully not included the `pattern="[0-9]*"` attribute here,
because the browser error messages conflict with our own and have
several drawbacks in terms of accessibility according to Adrian
Roselli's ["Avoid default field
validation"](https://adrianroselli.com/2019/02/avoid-default-field-validation.html).
Instead, the validation here will be part of the validation handling
later.
Also, I've opted for using `decimal` instead of `numeric`, because we
allow you to store decimal values and that inputmode also adds the
decimal separator to the keyboard. As always, however, there's
complications: several languages (including Norwegian) use a comma as a
decimal separator instead of a period, so the keyboard will likely
contain numbers and a comma instead of a period. This is a problem
because JS doesn't recognize "45,6" as a valid number. I've added a
follow-up task to look into this. I thought at first it would just be
expanding the validation, but because it's stored as a string on the
back end and the SDKs presumably parse it, we can't just suddenly allow
commas as decimal separators.
Adds an "add value" with popover input for single-value fields
(numerical and semver operators).
The implementation re-uses the popover from the multi-value constraint
operators, so I've extracted it for re-use.
All current input types:
<img width="779" alt="image"
src="https://github.com/user-attachments/assets/ad522e4d-72ba-402c-ad7c-8609ef2fb3a8"
/>
For the new one, opening the popover when there's a value will
pre-select the value, so you can override it by typing immediately:
<img width="297" alt="image"
src="https://github.com/user-attachments/assets/31d18f9e-6ef9-4450-9d63-ca5034b59f19"
/>
Buttons look pretty identical:
<img width="784" alt="image"
src="https://github.com/user-attachments/assets/d96b0b0d-0cbb-4262-9ca8-4ec919cbfafb"
/>
## Discussion points
### Input type
I haven't set an input type anywhere on the popover yet. In theory, we
could use input type "number" for numerical inputs and I think it's
worth looking at that, but we don't do in the old implementation either.
I've added a task for it.
### Weird esc handling
This implementation uses a chip for the button/value display for the
single. In almost all cases it works exactly as I'd expect, but closing
the popover with esc moves your focus to the top of `body`.
Unfortunately, this isn't something we can address directly (trust me,
I've tried), but the good news is that this was fixed in mui v6. The
current major is v7, so we probably want to update before too long,
which will also fix this. More info in the MUI docs:
https://mui.com/material-ui/migration/upgrade-to-v6/#chip
I think that for the single value entry, losing focus on esc is a fair
tradeoff because it handles swapping states etc so gracefully. For the
multi-value operators, however, esc is the only way to close the
popover, so losing focus when you do that is not acceptable to me. As
such, I'll leave the multi-value input as a button for now instead.
(It's also totally fine because the button never updates or needs to
change).
Fixes a few small styling issues with the constraint value chips:
- Background color was wrong
- They shouldn't have a border when they're not focused
Different styles:
1. Keyboard focus
2. Mouse hover
3. No focus
4. No focus
5. Add values button for reference.
<img width="405" alt="image"
src="https://github.com/user-attachments/assets/ded98393-a7a8-4d4a-81ff-63a3f4d32184"
/>
Fixes an issue with the new legal values selector where selecting an
item from filtering or changing the checkbox state would move your focus
to the top of the page. I think it's because we'd re-render the whole
tree because of it, and this would clear your focus selection. To get
around it, I've used the existing ResolveInput component. We might want
to change this later as we get around to more input components (single
values, etc), but for now, I think this is good enough.
As a bonus, I get to delete the most annoying part of the
EditableConstraints file 😄
The constraint still opens in edit mode for now, but I expect that to
get resolved once we properly implement the split between editable and
non-editable constraints that was started yesterday.
Removes the condition to hide the value list if we use legal values.
In doing so, I also realized that focus handling when you delete the
last item in the constraint values list doesn't work if the add values
button isn't there (which it shouldn't be for legal values and more). So
I've hidden the add values button when it doesn't do anythnig helpful
(or for cases where we don't have designs yet). In cases where you don't
have the add values button and you delete the last constraint value,
we'll move the focus to the "delete constraint" button (that was easier
than making sure we pass refs all the way down into the operator select,
but we can change that later).
To facilitate this (refs coming from the parent component), I refactored
the value list component to accept the add values widget as a child (and
extracted it to its own file).
Instead of closing the "add values" popover when you add a value, we now
keep it open to facilitate rapid entry of multiple values. It already
clears successfully and adds the new value to the list, so it's actually
quite smooth to use from just the keyboard now!
Additionally, I propose using a `form` element for the add values
popover, because it really is just a tiny form. This also allows us to
use regular form handling instead for submission instead of checking
what key the user pressed. It also means we don't need to specify the
action in the button, because the form handles it.
There's a few more things fixed:
- I've added a label (only visible to screen readers) to the input label
(as per standard a11y guidelines).
- When you add a value by pressing the "add" button, your focus returns
to the input field, so that you can just start typing out the next one.
this is handy if you submit by mouse click or by tabbing to the button
instead of just hitting enter inside the input field.
Code for constraint accordion was copy-pasted before previous
improvement. Old version is still in use for Segments. When we get to
improving constraint editing we should rebuild segments editing, without
use of this code.
