Implement lazy evaluation of attachTo.element

[monshan]

Internal review for #1187 to evaluate lazy attachment of attachTo.element

Changes:

• Remove
  • Step.isCentered()
• Write unit tests to evaluate shouldCenterStep
• Write cypress tests to evaluate parseAttachTo binding with multiple elements

---

Response to Considerations:

1. Should we get rid of Step.isCentered and is shouldCenterStep an appropriate alternative?

• Step.isCentered internally approved for removal
  • other methods do not deeply depend on isCentered
  • including shouldCenterStep to maintain the ability to evaluate the resolved attachTo options will not destroy the feature for any users that are currently or will want to use it in the future
• shouldCenterStep maintains and improves upon Step.isCentered
  • Testing written in general.spec.js to cover reasonable range of potential forms of resolved attachTo options
  • shouldCenterStep was modified to handle null and undefined results

2. Should Step._getResolvedAttachToOptions throw an Error if accessed before attachTo evaluation?

• Regarding 'Step._scrollTo' being called before _show
  • Found in testing that it does not destroy the feature and the handler is still called regardless of attachTo resolution timing
  • However will not produce likely desired results if called before _show should dynamic elements be added and removed per step options
  • More of a user behavioral change, recommended to disclose in the documentation
• Should Step._getResolvedAttachToOptions throw an error if accessed before the attachTo evaluation
  • Would only be material in the case that we would want to internally access the resolved attachTo options from any step other than the current step
  • setupTooltip will call this method to also determine a tooltip target, but as mentioned above this is a risk if we are concerned about surrounding steps while on the current step and that we call setupTooltip purely

3. Is lazy attachment potentially a breaking change?

• The main concern is there is only one opportunity to bind the step to the target vs what could be previously multiple opportunities to bind. This is especially important with dynamic content that may not exist at the beginning of the tour but be generated at the step of interest.
• Further cypress tests were written to check the behavior which include the following:
  • If by chance the attachTo element definition applies to multiple dynamic elements:
    • Only the first element matching the attachTo element definition will be assigned as the target
    • Any other elements that match the element definition if deleted will have no effect on the step as long as the target remains
    • If this targeted element is deleted after the show phase, the step focus and tooltip will still render normally as only the target has been removed
    • AttachTo options (resolved) are the same as non-dynamic content
  • If the whole step is removed (deleted), the attached element is also deleted from the DOM and cannot be found
  • If the whole step is not removed (deleted), the attached element still exists and can be found at any later point in the tour
• Major conclusions:
  • The binding is still successful regardless of the number of matching dynamic elements but only the first matching element will be considered the target
  • Even if the target is removed for any reason, as long as it happens post-show of the step the tour can continue as expected
  • Could be a breaking change for users that are binding to a single dynamic element and cannot render it before the show phase of the step in question even using the attachTo option element callback

4. Would this qualify as a major release?

• Though the breaking scenarios are niche at bare minimum the following is recommended:
  • Deprecation or removal of Step.isCentered (removal done)
  • A footnote in the docs that ._scrollTo should only be used after ._show for accurate behavior
  • The lazy attachTo can be accepted as long as the above considerations are approved

[RobbieTheWagner]

I think here we may need to have the element function return something that we change. Then make sure the options are the same still. I think the idea is not calling it twice in a row but calling it twice at various times, when different elements may be on the screen etc.

[RobbieTheWagner]