Allows for local step asynchronous events#1266
Allows for local step asynchronous events#1266ezra-obiwale wants to merge 4 commits intousablica:masterfrom
Conversation
- Removes duplicate loops - Implements step events
- Adds "no-title" class when a step doesn't have a title
- Adds the current step class ("step-[number]")
- Adds tests
| if (targetElement.title) { | ||
| oldTooltipHeaderLayer.classList.remove('no-title'); | ||
| } else { | ||
| oldTooltipHeaderLayer.classList.add('no-title'); |
There was a problem hiding this comment.
How this no-title class will be used? I couldn't find any CSS changes in this PR.
There was a problem hiding this comment.
Yeah, it's for themes to use if they need it.
For instance, in my use case, I needed to hide the header element if there's no title.
There was a problem hiding this comment.
Right, got it. I personally think we should keep the codebase simple and avoid adding any classes that is not used right now. I understand that you need that no-title class in your application though. Can you use your initial input (e.g. your steps list) to see if a specific step as title or not?
There was a problem hiding this comment.
Yeah, thought of that but that's JS. It's better to use CSS if possible instead of JS.
About avoiding adding classes, it's not unusual for libraries to add css classes that may help developers customize the outlook, even if the library doesn't use the classes.
| this._options.scrollTo, | ||
| disableInteraction, | ||
| }; | ||
| if (step > 0) { |
There was a problem hiding this comment.
This is a scary function to change mainly because we don't have any tests for it.
There was a problem hiding this comment.
Yeah, the point is to use a single loop, instead of two. As such, existing tests already cover this.
There was a problem hiding this comment.
Not sure if we have tests for this? there are two ways of adding the steps, using the data-* attributes and a JSON object. I don't think we have any tests for the data-* attr approach.
There was a problem hiding this comment.
I just started refactoring this function and adding some tests. I will ping you once it's merged.
|
I would be interested in this feature. |
3 participants
onbeforechange(next)andonchange()event callbacksBonus:
no-titleto the tooltip header layer if a step doesn't have a title