Pinging @elastic/es-ui (Team:Elasticsearch UI)

Hi @jloleysens , great work on converting edit policy to form lib! Tested locally and all seems to work, just a couple of questions to confirm/discuss:

1. do you think it would be possible/useful to type strings for path and key properties in UseField:

<UseField
        key={`phases.${phase}.actions.set_priority.priority`}
        path={`phases.${phase}.actions.set_priority.priority`}
...

2. Before, when a policy is created without a cold phase and the cold phase is enabled later, it would not have any index priority set. Now index priority is set to 50, is that intended? (I think there was 2 different 'default' values for phases before, when creating a new policy and when enabling a phase on an existing policy)
3. Not sure if that is a bug, but if I set min age to -1, save button is still enabled and 'Fix errors' badge only shown after the button is clicked. It seems to me that the intention in the code is to show the badge and to disable the button when the form is invalid. For me, form.isValid in form context is undefined, the error on the field itself is shown though.

Hey @yuliacech ! Thanks for the review!

1. I think it could be useful, but at this stage with the spec all defined in one place and CITs that cover the runtime behaviour I think the class of bugs that typing this paths gives is not massive. That being said, I came up with a solution for what something like that could look like:

interface MyInterface {
  a: {
    b: {
      c: {
        d: string;
      }
    }
  }
}

class PathBuilder<T> {
  constructor(protected parent?: PathBuilder<any>) {}

  property: keyof T;

  next(property: keyof T) {
    this.property = property;
    return new PathBuilder<T[typeof property]>(this);
  }

  private getPath() {
    const path: string[] = [];
    let parent: PathBuilder<any> = this.parent;
    while (parent !== undefined && parent.property) {
      path.push(String(parent.property))
      parent = parent.parent;
    }
    return path.reverse();
  }

  toString() {
    return this.getPath().join('.')
  }

}

const p = new PathBuilder<MyInterface>();


console.log(p.next('a').next('b').next('c').next('d').toString());
// a.b.c.d

Is this what you had in mind?

2. That is a good point! I had not checked that, perhaps it is best to leave this unset for now :)

3. This is the expected form behaviour. We will be changing this form overall quite soon I expect, I don't think we should be spending more effort now getting on parity with older behaviour. The isValid value of the form is only set explicitly after validate has been run which only happens when a form is submitted (or it is called in code somewhere explicitly). Field level validators run every time a field value changes.

@elasticmachine merge upstream

@yuliacech I addressed the default setting for index priority. Would you mind taking another look?

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: fba588d

Metrics [docs]

@kbn/optimizer bundle module count

async chunks size

distributable file count

page load bundle size

History

• 💚 Build #84238 succeeded c6671c6
• 💚 Build #83914 succeeded 0637d7d
• 💔 Build #83910 failed 7fff5a8

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Not blocking on review from elastic/kibana-design@ because this PR removes an SCSS file that was duplicated from a pre-existing file which was previously reviewed #76126