chore: more manual fixes

Author: timowestnosto

src/components/Popup/Popup.stories.tsx

@@ -31,6 +31,9 @@ Supports segment-based activation and persistent closure state using localStorag
 export default meta
 type Story = StoryObj
 
+// reset popular storage key for demo purposes
+localStorage.clear()
+
 export const Default: Story = {
   args: {},
   render: () => html`
@@ -43,13 +46,20 @@ export const Default: Story = {
         >
           Sign Up Now
         </button>
+        <button
+          n-ribbon
+          style="background: transparent; border: 1px solid #ccc; padding: 0.75rem 1.5rem; border-radius: 4px; cursor: pointer; margin-left: 0.5rem;"
+        >
+          Maybe Later
+        </button>
         <button
           n-close
           style="background: transparent; border: 1px solid #ccc; padding: 0.75rem 1.5rem; border-radius: 4px; cursor: pointer; margin-left: 0.5rem;"
         >
           Close
         </button>
       </div>
+      <div slot="ribbon">Hello from the ribbon!</div>
     </nosto-popup>
   `
 }

src/components/Popup/Popup.ts

@@ -56,46 +56,57 @@ export class Popup extends NostoElement {
 
   private handleClick(event: Event) {
     const target = event.target as HTMLElement
-    const toOpen = target?.matches(".ribbon") || target?.closest(".ribbon")
     const toClose = target?.matches("[n-close]") || target?.closest("[n-close]")
     const toRibbon = target?.matches("[n-ribbon]") || target?.closest("[n-ribbon]")
+    const toOpen = target?.matches("[slot='ribbon']") || target?.closest("[slot='ribbon']")
+    console.log("Popup clicked:", target, { toOpen, toClose, toRibbon })
 
     if (toOpen || toClose || toRibbon) {
       event.preventDefault()
       event.stopPropagation()
     }
-    if (toOpen) {
-      updateShadowContent(this, "open")
-    } else if (toClose) {
+    if (toClose) {
       closePopup(this)
+    } else if (toOpen) {
+      updateShadowContent(this, "open")
     } else if (toRibbon) {
       setPopupState(this.name, "ribbon")
       updateShadowContent(this, "ribbon")
     }
   }
 }
 
+const key = "nosto:web-components:popup"
+
+type PopupData = {
+  name: string
+  state: "open" | "ribbon" | "closed"
+}
+
 function initializeShadowContent(element: Popup, mode: "open" | "ribbon" = "open") {
   element.shadowRoot!.innerHTML = `
     <style>${popupStyles}</style>
-    <dialog ${mode === "open" ? "open" : ""} part="dialog" ${mode === "ribbon" ? 'class="hidden"' : ""}>
+    <dialog part="dialog">
       <slot name="default"></slot>
     </dialog>
     <div class="ribbon ${mode === "open" ? "hidden" : ""}" part="ribbon">
-      <slot name="ribbon"></slot>
+      <slot name="ribbon">Open</slot>
     </div>
   `
+  if (mode === "open") {
+    element.shadowRoot?.querySelector("dialog")?.showModal()
+  }
 }
 
 function updateShadowContent(element: Popup, mode: "open" | "ribbon" = "open") {
-  const dialog = element.shadowRoot?.querySelector("dialog")
+  const dialog = element.shadowRoot?.querySelector<HTMLDialogElement>("dialog")!
   const ribbon = element.shadowRoot?.querySelector(".ribbon")
   if (dialog && ribbon) {
     if (mode === "ribbon") {
-      dialog.classList.add("hidden")
+      dialog?.close()
       ribbon.classList.remove("hidden")
     } else {
-      dialog.classList.remove("hidden")
+      dialog.showModal()
       ribbon.classList.add("hidden")
     }
   }
@@ -110,25 +121,19 @@ async function getPopupState(name: string, segment?: string): Promise<"open" | "
   if (segment && !(await checkSegment(segment))) {
     return "closed"
   }
-  const key = getKey(name)
-  const state = localStorage.getItem(key)
-  if (state === "closed" || state === "ribbon") {
-    return state
+  const dataStr = localStorage.getItem(key)
+  if (dataStr) {
+    const data = JSON.parse(dataStr) as PopupData
+    if (data.name !== name) {
+      return "closed"
+    }
+    return data.state
   }
   return "open"
 }
 
 function setPopupState(name: string, state: "open" | "ribbon" | "closed") {
-  const key = getKey(name)
-  if (state === "open") {
-    localStorage.removeItem(key)
-  } else {
-    localStorage.setItem(key, state)
-  }
-}
-
-function getKey(name: string) {
-  return `nosto:web-components:popup:${name}`
+  localStorage.setItem(key, JSON.stringify({ name, state }))
 }
 
 async function checkSegment(segment: string) {

src/components/Popup/styles.ts

@@ -19,11 +19,11 @@ export const popupStyles = `
     padding: 0;
     background: transparent;
     pointer-events: auto;
-    z-index: 1001;
   }
 
   dialog::backdrop {
-    background: rgba(0, 0, 0, 0.5);
+    background: rgba(0, 0, 0, 0.65);
+    backdrop-filter: blur(2px);
   }
 
   .ribbon {

test/components/Popup/Popup.spec.tsx

@@ -5,6 +5,16 @@ import { mockNostojs } from "@nosto/nosto-js/testing"
 import { createElement } from "../../utils/jsx"
 
 describe("Popup", () => {
+  const popupKey = "nosto:web-components:popup"
+
+  function getPopupData() {
+    return JSON.parse(localStorage.getItem(popupKey)!)
+  }
+
+  function setPopupData(data: { name: string; state: "open" | "ribbon" | "closed" }) {
+    localStorage.setItem(popupKey, JSON.stringify(data))
+  }
+
   beforeEach(() => {
     // Clear localStorage before each test
     localStorage.clear()
@@ -58,7 +68,7 @@ describe("Popup", () => {
   describe("Named popups and persistence", () => {
     it("should hide popup if it was previously closed and name is set", async () => {
       const popupName = "test-popup"
-      localStorage.setItem(`nosto:web-components:popup:${popupName}`, "closed")
+      setPopupData({ name: popupName, state: "closed" })
 
       const popup = (
         <nosto-popup name={popupName}>
@@ -208,7 +218,7 @@ describe("Popup", () => {
       const closeButton = popup.querySelector("[n-close]") as HTMLButtonElement
       closeButton.click()
 
-      expect(localStorage.getItem(`nosto:web-components:popup:${popupName}`)).toBe("closed")
+      expect(getPopupData()).toEqual({ name: popupName, state: "closed" })
     })
 
     it("should always store closed state in localStorage since name is required", async () => {
@@ -226,7 +236,7 @@ describe("Popup", () => {
       const closeButton = popup.querySelector("[n-close]") as HTMLButtonElement
       closeButton.click()
 
-      expect(localStorage.getItem("nosto:web-components:popup:always-stores-popup")).toBe("closed")
+      expect(getPopupData()).toEqual({ name: "always-stores-popup", state: "closed" })
     })
 
     it("should handle click events on ribbon content with n-close", async () => {
@@ -249,7 +259,7 @@ describe("Popup", () => {
       ribbonCloseButton.click()
 
       expect(popup.style.display).toBe("none")
-      expect(localStorage.getItem("nosto:web-components:popup:ribbon-popup")).toBe("closed")
+      expect(getPopupData()).toEqual({ name: "ribbon-popup", state: "closed" })
     })
 
     it("should not close popup when clicking elements without n-close attribute", async () => {
@@ -332,7 +342,7 @@ describe("Popup", () => {
       const popupName = "segment-popup"
 
       // First, close the popup
-      localStorage.setItem(`nosto:web-components:popup:${popupName}`, "closed")
+      setPopupData({ name: popupName, state: "closed" })
 
       // Set up segments that would normally show the popup
       mockNostojs({
@@ -396,18 +406,18 @@ describe("Popup", () => {
       ribbonButton.click()
 
       // Check that ribbon state is stored
-      expect(localStorage.getItem("nosto:web-components:popup:ribbon-test-popup")).toBe("ribbon")
+      expect(getPopupData()).toEqual({ name: "ribbon-test-popup", state: "ribbon" })
 
       // Check DOM structure after switch
       const dialog = popup.shadowRoot?.querySelector('[part="dialog"]')
       const ribbon = popup.shadowRoot?.querySelector('[part="ribbon"]')
-      expect(dialog?.classList.contains("hidden")).toBe(true)
+      expect(dialog?.hasAttribute("open")).toBe(false)
       expect(ribbon?.classList.contains("hidden")).toBe(false)
     })
 
     it("should render in ribbon mode when localStorage state is 'ribbon'", async () => {
       const popupName = "persistent-ribbon-popup"
-      localStorage.setItem(`nosto:web-components:popup:${popupName}`, "ribbon")
+      setPopupData({ name: popupName, state: "ribbon" })
 
       const popup = (
         <nosto-popup name={popupName}>
@@ -421,7 +431,7 @@ describe("Popup", () => {
 
       const dialog = popup.shadowRoot?.querySelector('[part="dialog"]')
       const ribbon = popup.shadowRoot?.querySelector('[part="ribbon"]')
-      expect(dialog?.classList.contains("hidden")).toBe(true)
+      expect(dialog?.hasAttribute("open")).toBe(false)
       expect(ribbon?.classList.contains("hidden")).toBe(false)
     })
 

test/setup.ts

@@ -11,6 +11,13 @@ import "@/components/SectionCampaign/SectionCampaign"
 import "@/components/SimpleCard/SimpleCard"
 import "@/components/SkuOptions/SkuOptions"
 
+HTMLDialogElement.prototype.showModal = function () {
+  this.toggleAttribute("open", true)
+}
+HTMLDialogElement.prototype.close = function () {
+  this.toggleAttribute("open", false)
+}
+
 beforeAll(() => {
   // Components are automatically registered by their @customElement decorators
   // when the modules are imported above.