nlxai / sdk

The NLX SDK
https://nlxai.github.io/sdk
MIT License
6 stars 0 forks source link

ADR: highlights in journey manager #149

Open michaelglass opened 1 month ago

michaelglass commented 1 month ago

The JourneyManager allows customers to highlight targets to move the customer along the flow.

We implemented highlights two ways: overlay and box-shadow.

This ADR discusses how these two compare, what other options might be, and why we've decided for box-shadow


overlay

First, in order to avoid changing the customer's DOM, we implemented highlights by creating a new div in the journey manager's shadow DOM, copying over styles from the target element so the div looks similar to the target element, and floating it using z-index above the target element.

pros

is not affected by changes to customer's DOM.

cons

sometimes peaks through full-screen modals because it's impossible to insert our divs in between z-indeces of sibling stacking-contexts. For example: using the overlay approach, it is impossible to place the highlight between divs 5 and 6: image

box-shadow

To solve the modal issue of the overlay approach, @jakub-nlx implemented highlights by directly applying styles to the target element.

pros

cons

may have unintended conflicts with customer's DOM.

Unimplemented approach: overlay directly on the target

Rather than using the Journey Manager's shadow DOM, we can add a shadow dom directly on the target element and create the overlay from within it.

Pros

avoid stacking context issues

cons

Conclusion

The box-shadow approach is the most robust solution we've implemented. As we learn about specific concerns from usage, we may revive one of the other two overlay approaches if we can't address them in the box-shadow approach.

jakub-nlx commented 1 month ago

@peterszerzo does this mean we should remove the overlay code?

peterszerzo commented 1 month ago

@jakub-nlx I would prefer to wait until we get some production usage, that should help us figure out how much these pros and cons matter.

peterszerzo commented 1 month ago

But in any case, work on overlay improvements is stopping for now, this issue was just meant to document the findings.