This (admittedly pretty big) PR removes a component layer, moves all
logic for updating constraint values into a single module, and dumbs
down other components.
The main changes are:
- EditableConstraintWrapper is gone. All the logic in there has been
moved into the new `useEditableConstraint` hook. Previously it was split
between the wrapper, editableConstraint itself, the legalValues
component.
- the `useEditableConstraint` hook accepts a constraint and a save
function and returns an editable version of that constraint, the
validator for input values, a function that accepts update commands,
and, when relevant, existing and deleted legal values.
- All the logic for updating a constraint now exists in the
`constraint-reducer` file. As a pure function, it'll be easy to unit
test pretty thoroughly to make sure all commands work as they should
(tests will come later)
- The legal values selector has been dumbed down consiberably as it no
longer needs to create its own internal weak map. The internal
representation of selected values is now a set, so any kind of lookup is
now constant time, which should remove the need for the extra layer of
abstraction.
## Discussion points
I know the reducer pattern isn't one we use a *lot* in Unleash, but I
found a couple examples of it in the front end and it's also quite
similar to how we handle state updates to change request states. I'd be
happy to find a different way to represent it if we can keep it in a
single, testable interface.
Semi-relatedly: I've exposed the actions to submit for the updates at
the moment, but we could map these to functions instead. It'd make
invocations a little easier (you wouldn't need to specify the action
yourself; only use the payload as a function arg if there is one), but
we'd end up doing more mapping to create them. I'm not sure it's worth
it, but I also don't mind if we do 💁🏼
This is a helper PR for a refactor I'm working on for the new constraint
inputs. In the refactoring, it's useful to have individual subtypes for
the various subgroups of operators and to be able to easily assert
whether something is X operator or not.
The only change required in the code base is a single check for
operators, which is now handled by using the new `isXOperator` functions
instead.
Yes, the operator file in constants now includes functions, but it
seemed useful to put the identification functions there instead of
somewhere unrelated. The tests are primarily to ensure that the
identifier function works, and I'd be happy to remove them if we think
it's necessary. That said, they're pretty simple unit tests, so I think
it's fine to leave them.
The main bulk of the change is: removing the explicit `: Operator[]`
typing to the various sub-sets of operators and instead adding explicit
types. Additionally, there's the new identifier functions.
Did two things:
1. Use basePath to prefix the Unleash repo used with the Unleash
Frontend SDK.
2. Use JSON.stringify as key for useEffect to avoid potential render
loop
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"
/>
Initial PR that adds logic for displaying a link to stripe to view
consumption based pricing in the billing overview
---------
Co-authored-by: Nuno Góis <github@nunogois.com>
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"
/>
