search

tjek / tjek-js-sdk

Tjek JavaScript SDK for the browser and Node.js.
18 stars 4 forks source link

Jl/page decorations improvements #225

Closed jrlarano closed 6 months ago

jrlarano commented 8 months ago

Note: PageDecorations should be passed on applyHotspots to render its hotspost similar to normal hotspots

Eg: bootstrapper.applyHotspots(sgnViewer, hotspots, sgnPageDecorations);

jrlarano commented 7 months ago

Hi @mortenbo, can we merge this now?

mortenbo commented 7 months ago

It's in prod now but I have one change needed: link and embed_link are now strings. So if embed_link is set, use that. If else link is set, use that.

Screenshot 2024-04-15 at 16 41 31
jrlarano commented 7 months ago

@mortenbo here's how it should look like now 🙂

image

mortenbo commented 7 months ago

@jrlarano Super, what about if link?

jrlarano commented 7 months ago

Link is like this 🙂 image

mortenbo commented 7 months ago

OK, great. We need to fix this: https://github.com/tjek/tjek-js-sdk/pull/225/files#diff-56e8833b30816a4984798688ef87a98d9e5093e18e1ba083ebe1446d64b54e9bR75. You need to set sandbox="allow-scripts allow-same-origin"

mortenbo commented 7 months ago

Otherwise, it's fine to merge and deploy if @klarstrup agrees with the PR.

mortenbo commented 7 months ago

@klarstrup Once deployed, I'd like to get this into Vizsla too.

mortenbo commented 6 months ago

Is this a regression?

Screenshot 2024-04-19 at 10 50 35
klarstrup commented 6 months ago

no that test just sucks