feat(rollouts/rules): UI for rollout/rule editing of segments #1953
Conversation
Codecov Report
@@ Coverage Diff @@
## segment-anding #1953 +/- ##
===============================================
Coverage 70.85% 70.85%
===============================================
Files 67 67
Lines 6681 6681
===============================================
Hits 4734 4734
Misses 1672 1672
Partials 275 275 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
yquansah
force-pushed
the
edit-segment-anding
branch
from
aec40e9
to
01f0d7e
Compare
yquansah
force-pushed
the
segment-anding
branch
3 times, most recently
from
e2f936a
to
8c1d4bc
Compare
yquansah
force-pushed
the
edit-segment-anding
branch
from
01f0d7e
to
0c3af94
Compare
yquansah
force-pushed
the
edit-segment-anding
branch
from
87d09cd
to
a4c37a3
Compare
| @@ -51,7 +54,9 @@ export default function Combobox<T extends IFilterable>( | |||
| <div className="relative flex w-full flex-row"> | |||
| <C.Input | |||
| id={id} | |||
| className="text-gray-900 bg-gray-50 border-gray-300 w-full rounded-md border py-2 pl-3 pr-10 shadow-sm focus:border-violet-500 focus:outline-none focus:ring-1 focus:ring-violet-500 sm:text-sm" | |||
| className={twMerge( | |||
There was a problem hiding this comment.
does twMerge replace the classNames util helper?
There was a problem hiding this comment.
@markphelps I think so? Was the classNames util helper for handling overrides of tailwind styles as well? If so, it didn't seem to be working for me for some reason 🤔. This article explains why twMerge should be used.
There was a problem hiding this comment.
i think twMerge is like the functionality of classNames + handles tailwind specifically. classNames is just a way to dynamically apply classes given a boolean condition. so it might make sense for us to favor twMerge instead of classNames in the future as it has the added ability of working with tw classes like we might expect (last one wins).
So maybe the rule should be, if we find ourselves needing to use classNames + twMerge then just use twMerge as it can do both I think?
ui/src/components/forms/Combobox.tsx
Outdated
| @@ -22,6 +24,7 @@ export default function Combobox<T extends IFilterable>( | |||
| const { | |||
| id, | |||
| className, | |||
| inputClassNames, | |||
There was a problem hiding this comment.
| inputClassNames, | |
| inputClassName, |
ui/src/components/forms/Combobox.tsx
Outdated
| @@ -14,6 +15,7 @@ type ComboboxProps<T extends IFilterable> = { | |||
| setSelected?: (v: T | null) => void; | |||
| disabled?: boolean; | |||
| className?: string; | |||
| inputClassNames?: string; | |||
There was a problem hiding this comment.
| inputClassNames?: string; | |
| inputClassName?: string; |
| {(!editing || parentSegments.length === 0) && ( | ||
| <div className="w-full"> | ||
| <div className="w-5/6"> | ||
| <Combobox<FilterableSegment> |
There was a problem hiding this comment.
should we set disabled=${readOnly} here?
There was a problem hiding this comment.
Do you mean like this?
Or something else?
There was a problem hiding this comment.
yah, nvm i see that the above is only rendered when in 'editing' mode
UI For edits/updates to rule/rollout segments.
Completes FLI-537