FIO-9241: Correctly set current page after conditionally hiding page in sibling nested wizard

[blakekrammes]

Link to Jira Ticket

https://formio.atlassian.net/browse/FIO-9241

Description

What changed?

I updated the onChange() method in the Wizard class to call this.setPage() if the number of total pages in the parent/root-level wizard changes, which updates the current page of the root wizard to be the current page/page (i.e. this.currentPanel. this.setPage() was not being called onChange() for wizards with sub-wizards. I also updated render() to set this.currentPanel based on id and not key.

Why have you chosen this solution?

I changed onChange() in the way that I did to simplify the logic for when to set the current page and to ensure that the correct page is selected based on a (more) unique id and not key, which could be easily duplicated between sub-wizard pages.

For the update to render(), I made that change since another bug manifested where pages in multiple sub-wizards having the same key would cause the current page to jump from one sub-wizard to another. (i.e. B2 to C2) when A2 was conditionally hidden by the value of B2 because both B2 and C2 have the same key of (I think) 'page2'. A2 being a page of sub-wizard form A, and the same for pages B2 and C2, in respect to the sub-wizards B and C.

Breaking Changes / Backwards Compatibility

I removed this.currentPanels from the Wizard class in order to simplify the logic in onChange() since the current Wizard pages/panels (including those of all sub-wizards) can be accessed via this.pages. This might break something, but I'm not sure of a case where it would. All of the tests pass with it removed. See this comment for more details.

Dependencies

n/a

How has this PR been tested?

Manual and automated testing.

Checklist:

• I have completed the above PR template
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation (if applicable)
• My changes generate no new warnings
• My changes include tests that prove my fix is effective (or that my feature works as intended)
• New and existing unit/integration tests pass locally with my changes
• Any dependent changes have corresponding PRs that are listed above

[blakekrammes]

There was an additional bug here:

If you had multiple/sibling nested wizards and conditional logic controlling the visibility of a nested wizard page, this was setting the currentPanel to the wrong page on render (i.e. both forms have a 'page1' key and the wizard would navigate to 'page1' on nested form C instead of 'page1' on nested form B. This is fixed by using a unique id to set the currentPanel.

[blakekrammes]

this.currentPanels was only referenced within onChange and seems like an unnecessary piece of state given that the current pages/panels can be accessed via this.pages. I could be missing/unaware of a case, but all the tests pass happily without it and I really wanted to simplify this logic.

From what I can tell, this.currentPanels was only tracking the parent pages, which in the case of a wizard having nested wizards forms A, B and C, would only return the top-level parent pages for those forms. However, that's not useful when you're trying to update the current page to a particular page in one of those nested forms. I don't believe it's possible to navigate to a parent page A without expanding its children and selecting the first page A1. this.pages is a flattened array and stores every page in every nested wizard and can be used to select the correct page after onChange is called for both simple, single-level wizards, and wizards with nested wizards.

I think the reason why this.currentPanels worked for selecting the current page in single-level wizards was because the top-level panel components are navigable pages. However, when you have nested wizards, they are not navigable pages.

For comparison:

this.currentPanels = [{ title: 'A', ... }, { title: 'B', ... }, { title: 'C', ... }]
this.pages = [{ title: 'A1', ... }, { title: 'A2', ... },  { title: 'A3', ... }, { title: 'B1', ... }, ...]

[blakekrammes]

.forEach is more appropriate since we're applying side-effects and not returning a new array.