refactor: fix strategy modal issues (#778)

* refactor: wait for contexts before rendering constraints

* refactor: fix constraints list index

* refactor: move add constraint button for improved focus

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints2/FeatureStrategyConstraints2.styles.ts

@@ -0,0 +1,8 @@
+import { makeStyles } from '@material-ui/core/styles';
+
+export const useStyles = makeStyles(theme => ({
+    container: {
+        display: 'grid',
+        gap: '1rem',
+    },
+}));

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints2/FeatureStrategyConstraints2.tsx

@@ -10,6 +10,8 @@ import {
 } from 'component/providers/AccessProvider/permissions';
 import { createEmptyConstraint } from 'component/feature/FeatureStrategy/FeatureStrategyConstraints2/createEmptyConstraint';
 import { useWeakMap } from 'hooks/useWeakMap';
+import { objectId } from 'utils/object-id';
+import { useStyles } from 'component/feature/FeatureStrategy/FeatureStrategyConstraints2/FeatureStrategyConstraints2.styles';
 
 interface IFeatureStrategyConstraints2Props {
     projectId: string;
@@ -37,6 +39,7 @@ export const FeatureStrategyConstraints2 = ({
     const state = useWeakMap<IConstraint, IConstraintFormState>();
     const { context } = useUnleashContext();
     const { constraints = [] } = strategy;
+    const styles = useStyles();
 
     const onEdit = (constraint: IConstraint) => {
         state.set(constraint, { editing: true });
@@ -79,21 +82,12 @@ export const FeatureStrategyConstraints2 = ({
         );
     };
 
+    if (context.length === 0) {
+        return null;
+    }
+
     return (
-        <>
-            {strategy.constraints?.map((constraint, index) => (
-                <ConstraintAccordion
-                    key={index}
-                    environmentId={environmentId}
-                    constraint={constraint}
-                    onEdit={onEdit.bind(null, constraint)}
-                    onCancel={onCancel.bind(null, index, constraint)}
-                    onDelete={onRemove.bind(null, index, constraint)}
-                    onSave={onSave.bind(null, index)}
-                    editing={Boolean(state.get(constraint)?.editing)}
-                    compact
-                />
-            ))}
+        <div className={styles.container}>
             <PermissionButton
                 type="button"
                 onClick={onAdd}
@@ -104,6 +98,19 @@ export const FeatureStrategyConstraints2 = ({
             >
                 Add constraints
             </PermissionButton>
-        </>
+            {strategy.constraints?.map((constraint, index) => (
+                <ConstraintAccordion
+                    key={objectId(constraint)}
+                    environmentId={environmentId}
+                    constraint={constraint}
+                    onEdit={onEdit.bind(null, constraint)}
+                    onCancel={onCancel.bind(null, index, constraint)}
+                    onDelete={onRemove.bind(null, index, constraint)}
+                    onSave={onSave.bind(null, index)}
+                    editing={Boolean(state.get(constraint)?.editing)}
+                    compact
+                />
+            ))}
+        </div>
     );
 };

frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints2/createEmptyConstraint.ts

@@ -4,6 +4,10 @@ import { IConstraint } from 'interfaces/strategy';
 export const createEmptyConstraint = (
     context: IUnleashContextDefinition[]
 ): IConstraint => {
+    if (context.length === 0) {
+        throw new Error('Expected at least one context definition');
+    }
+
     return {
         contextName: context[0].name,
         operator: 'IN',

frontend/src/utils/object-id.test.ts

@@ -0,0 +1,13 @@
+import { objectId } from 'utils/object-id';
+
+test('objectId', () => {
+    const a = {};
+    const b = {};
+    const c = {};
+    expect(objectId(a)).toBeGreaterThan(0);
+    expect(objectId(b)).toBeGreaterThan(0);
+    expect(objectId(c)).toBeGreaterThan(0);
+    expect(objectId(a)).toBe(objectId(a));
+    expect(objectId(a)).not.toBe(objectId(b));
+    expect(objectId(a)).not.toBe(objectId(c));
+});

frontend/src/utils/object-id.ts

@@ -0,0 +1,12 @@
+// Get a unique ID for an object instance.
+export const objectId = (value: object): number => {
+    if (!ids.has(value)) {
+        id++;
+        ids.set(value, id);
+    }
+
+    return ids.get(value)!;
+};
+
+const ids = new WeakMap<object, number>();
+let id = 0;