Refactor CSS classes of wizard step indicators #181
Conversation
…ptional > done > current > current.done
See also comments in the CSS code.
| // Removing this definition now will allow to navigate to every step the user is able to navigate with the [awGoToStep] directive. | ||
| // For example, right after opening the wizard with basic configuration, the user would be able to go to the second step | ||
| // by clicking its indicator in the navigation bar instead of using the [Next] button. This behavior is undesireable. | ||
| pointer-events: auto; |
There was a problem hiding this comment.
Mind the comment. I wanted to get rid of the pointer-events styling on my way, but could not.
There was a problem hiding this comment.
Hmmm, I'm not sure I understand the problem.
I mean the current navigation mode interface contains two methods to check whether a navigation from a current step to a destination step can be performed:
canGoToStep: this method is called for "normal" transitions, which are normally performed when the navigation directives are usedisNavigable: this method performs an additional check for the navigation using the navigation bar
To disable the navigation using the navigation bar we should just need to improve the isNavigable method, right?
There was a problem hiding this comment.
To disable the navigation using the navigation bar we should just need to improve the
isNavigablemethod, right?
The method itself is fine, the problem is that it is only called to set CSS classes. It is not used when a user clicks a step indicator, and I believe it should be.
When checking whether the user can navigate to a step, we should not rely on CSS classes that are supposed to set pointer-events: none to silence the [awGoToStep] directive (the one used by the navigation bar internally). Instead, we should probably make the [awGoToStep] conditional so that it only takes effect when needed, no matter what is happening on the presentation level (CSS).
Perhaps, users will also find use cases for conditional goTo*Step directives.
Having said that, I think the best action now is to remove this comment and leave pointer-events: none intact because this change:
- has little to do with the purpose of this PR and is better to be done separately / later
- has no purpose from the perspective of the end users.
- will affect the
NavigationMode's part of the code source code, which is already being refactored in Refactor the hierarchy of NavigationMode classes. #166 and Add step change and disabled step option #178, and will complicate matters even further.
Shall I remove the comment?
There was a problem hiding this comment.
I agree.
Let's remove the comment and solve the problem in another PR/issue.
|
Note that the bug from #182 is carefully preserved. |
…ed on the completion status of the whole wizard
|
I'll take a detailed look at the changes of the PR tomorrow |
src/lib/components/wizard-navigation-bar.component.horizontal.less
Outdated
Show resolved
Hide resolved
Conflicts: src/lib/components/wizard-navigation-bar.component.spec.ts src/lib/components/wizard-navigation-bar.component.ts
earshinov
changed the title
|
Sorry, I forgot that I have actually updated the vertical layout as well :-/ |
|
Hello @madoar , Sorry I have missed your earlier comments on my changes. Anyway, 4.0.0 is great news for us, because thanks to clickable step indicators and May I ask what plans you have for angular-archwizard for the nearest future? It would be nice if we could merge either #181 (this issue) or #166 . |
earshinov
changed the title
|
@earshinov thank you for your updates! For the next release of
In addition I'm thinking about including some of the following features: If you have any other changes you would like to see feel free to open an issue or a PR! |
|
@earshinov do you know if the demo repository is compatible with the changes introduced by this PR? |
|
@madoar , Custom step CSS example in the demo needs to be updated. |
Please see #124 for the relevant discussion.