Incorrect hover behavior of step indicators after the completion step has been visited

[earshinov]

Scenario:

• In the wizard with the competion step, click though the steps until you reach the completion step.
• Observe that indicators for previous steps react to hover, but clicking them does not cause navigation:

The issue can be reproduced on the demo page:
https://angular-archwizard-css-classes.stackblitz.io/

Pulled out from #124 (comment)

[earshinov]

From the implementation perspective, fixing this bug is not obvious and easy.

It seems that NavigationBarComponent.isNavigable should reach as far as to CompletionWizardStep.canExit. The implementation of NavigationMode.isNavigable should ideally call canGoToStep, which takes account of wizardStep.canExit, but canGoToStep is async and isNavigable should be sync.

I can only suggest splitting canGoToStep into two methods S (sync) and A (async). NavigationMode.isNavigable would then call S, and S would call WizardStep.canExit (which happens to be sync). @madoar , does it sound reasonable to you?

This bug seems to me a blocker for the next release.

[madoar]

WizardStep.canExit and WizardStep.canEnter are not completely synchronous.
Both inputs allow for the signature (direction: MovingDirection) => Promise<boolean>.
So basically the method itself is synchronous but the the evaluation of the return value is not.
Both WizardStep.canExit and WizardStep.canEnter need to be used by both the synchronous and the asynchronous canGoToStep methods, I'm not sure whether this is possible?

[madoar]

@earshinov do you think it makes sense to add an additional check to the isNavigable method, which checks whether the wizard is completed? If the wizard is completed isNavigable should return always false.

[earshinov]

@madoar , This change makes perfect sense, because the user should not navigate around in a complete wizard. Also, I believe it will fix this issue.

One downside is that a decision whether navigation should be allowed or not, will be made in some way or another in 3 (!) components:

1. WizardStep (CompletionWizardStep sets canExit to false)
2. NavigationMode
3. NavigationBar (after the change)

It makes responsibility scattered. Ideally, only NavigationMode should be responsible.

Having said that, from the practical standpoint it is better to make the change, cover it with tests, and apply our perfectionism and refactoring skills some time later.

[madoar]

I'm not sure we can move the navigation responsibility to only one class:

• WizardStep is the place where canExit and canEnter are specified, because these are the only components which the user has direct access to (by creating them)
• NavigationMode provides the complete "navigation logic"
• NavigationBar represents the navigation bar which is hidden from the user and needs to contain some checks to add the correct css classes for each wizard step

I'm not sure whether my suggested quick bug fix is really as easy at it seems. After thinking about this a bit more I discovered that this quick fix would lead to undesired behavior in case of a <awWizardCompletionStep awEnableBackLinks>. If this pair occurs the wizard should support leaving the completion step, which would not be possible anymore with my quick fix.

[earshinov]

Let's start with a unit test

[madoar]

Good idea!