The current implementation states that if you do not have
access—typically as a viewer user—it renders the old component.
Rendering an empty view is acceptable, as this is part of the segment
creation flow, and if you do not have permissions, you do not need to
see it.
Adds a test suite for the useEditableConstraint hook, attempting to test
all the parts of it that we can't test in isolation.
Plus: a few, small refactorings:
- Renames `onAutoSave` on `onUpdate` to better match `onDelete` (and
because autosave doesn't really mean anything anymore).
- Simplifies and collapses some types
Adds a fairly comprehensive test suite for the constraint reducer. I put
in all cases I thought were relevant.
As part of this, I discovered one bug, and changed two actions.
The bug was toggling on the wrong property when you tried to invert case
sensitivity.
The action changes are:
- rename "remove value from list" to "remove value"
- remove "set value" in favor of instead letting "add value(s)" work in
single-value constraints too.
Extracts and tests the implementation of the functions to get deleted
and invalid legal values.
This is pretty straightforward stuff and arguably not crucial, but it
was an easy place to start and I don't think it hurts to have these in
place anyway.
This will stop displaying the semver version for our hosted (i.e.
managed) customers, as it has become less relevant over time. It remains
valuable for self-hosted users.
Adds an input form for single legal values (i.e. for number and semver
operators).
- Uses the `validator` for the constraint to check the list of legal
values and provides a list of "invalid legal values" for values that
don't pass validation.
- Values in the "invalid legal values" set are "disabled" when rendered
in the UI. Additionally, there's an extra bit of text that tells you
that values that aren't valid are disabled.
- Makes the legal values selector more generic and makes it adapt to
multi- or single-value selection based on input props. The external
interface is two separate components (to make it clearer that they are
different things and because their props don't line up perfectly).
Rendered:
<img width="957" alt="image"
src="https://github.com/user-attachments/assets/cd8d2f32-057d-4e31-8fd3-174676eeb65e"
/>
Still todo: testing deleted/invalid legal value detection. I'll do that
as part of the big testathon in a followup.
Fixes an issue where you would lose keyboard focus when interacting with
one of the input fields in the new editable constraint.
The fix was spinning out the inputs into their own separate component.
This prevents them from being re-rendered every time (or something idk)
which allows us to keep focus.
It also stops the popover for multi-value constraint operators from
closing after you've entered a value; allowing for faster entry of more
values (as was intended and as was how it functioned previously).
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"
/>
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.
Use event.preventDefault to prevent the app from trying to submit the
legal values (or the strategy) form when you hit "enter" in the legal
values filter input.
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.
## About the changes
Currently, we're running against the older version of our UI. When
making changes to it we want to make sure we're testing the current code
**Details in comments**
---------
Co-authored-by: Tymoteusz Czech <2625371+Tymek@users.noreply.github.com>
You should not be able to break initial page redirect even if you set
'/' as target. It is not strictly needed in the current code path. This
will create a redirect loop only if you manually modify local storage.
It just makes this part safer if it is ever modified.
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.
https://linear.app/unleash/issue/2-3512/bug-flow-2-enable-for-a-specific-user-doesnt-workhttps://linear.app/unleash/issue/2-3513/bug-flow-4-adjust-variants-doesnt-work
Fixes our demo topics by making them work with the new flag page design.
We achieve this by adding 2 new interactive steps in `demoApp.step2` and
`demoApp.step4` to expand the respective environment accordions. We mark
them as optional so they are not strictly required and will be skipped
if not found. This means the demo will be resilient to rolling back the
`flagOverviewRedesign` flag, for example.
We also mark 2 steps as optional: saving constraints in `demoApp.step2`
and `demoApp.step4`. It seems like we no longer have an extra button to
save the constraints after adding them, so by marking these steps as
optional the demo flow is able to proceed without breaking.
Adds the easy parts of the inline values list: a list of chips that
shows you which values you have and that you can delete. You either
delete them by clicking the "clear" icon or by using del/backspace on
your keyboard.
If you use your keyboard we also handle switching your focus to the
appropriate element. By default, your browser may shift the focus to the
top of the window (which isn't very helpful). Instead, we handle it like
this:
- If you delete an item and there are more elements in the list:
- move the element to the next item if exists
- if your element is the last item, move focus to the previous item
- if there are no more items in the list, move the focus to the Add
Values button
We still need to add the "add values" popover functionality. That's next
on the agenda.
Additionally, this switches how the containing flex container positions
its items along the cross axis (vertically) to "flex-start" instead of
"center". Because the values list can grow to multiple lines, it would
shift the "delete constraint" button and the constraint picker to the
middle of the expanded constraint. Now, instead they stay aligned to the
top. This causes a slight alignment issue with the button (due to the
invisible padding), but I don't want to look at that before the rest of
this is complete and we know how it all fits together. You'll notice
that the spacing between elements in that top row is also off anyway
(look at the value list being smushed up against the case sensitive
icon), so there's more work to do.
<img width="716" alt="image"
src="https://github.com/user-attachments/assets/225fcab8-03e4-46e3-92d4-82912eb40d46"
/>
Focus styles:
<img width="190" alt="image"
src="https://github.com/user-attachments/assets/6b07ab25-0a67-493c-9cac-839932b0d654"
/>
<img width="195" alt="image"
src="https://github.com/user-attachments/assets/9d5b323e-bf65-4eca-9008-a45ce0139a2b"
/>
Hover styles:
<img width="96" alt="image"
src="https://github.com/user-attachments/assets/f19e1945-d2be-4e87-8005-76cb6beb1f50"
/>
In this PR I integrate the Unleash React SDK with the Admin UI.
We also take advantage of Unleash Hosted Edge behind the scenes with
multiple regions to get the evaluations close to the end user.
Implements the first step towards implementing the new design for
constraint editing. All the edit functionalities work as and when you do
them now, but there is no validation of the values you put in that's
happening.
The inverted / not inverted button and the case sensitivity button are
placeholders. They should use icons and have proper descriptions of what
they do. I'll do that in a follow-up.
The way to enter values is currently always in the section below the
main controls. Again, more work on this is coming.
Current look:
With case sensitive options:
<img width="769" alt="image"
src="https://github.com/user-attachments/assets/bfdfbac1-cc95-4f26-bf83-277bae839518"
/>
With legal values:
<img width="772" alt="image"
src="https://github.com/user-attachments/assets/14f566cc-d02a-46dd-b433-f8b13ee55bcc"
/>
