scale icon indicating copy to clipboard operation
scale copied to clipboard

Notification Message: Dismissing a message inside a modal triggers scale-close of modal

Open georgwittberger-telekom-mms opened this issue 1 year ago • 2 comments

Scale Version

3.0.0-beta.134

Framework and version

React 18.2.0 with @telekom/scale-components-react

Current Behavior

When dismissing a <scale-notification> message displayed inside of a <scale-modal> the event handler scale-close of the modal is triggered as well. Typically, this causes the modal to close because it changes an external state which is bound to the opened prop.

Expected Behavior

When dismissing a <scale-notification> message displayed inside of a <scale-modal> the event handler scale-close of the modal is not triggered.

Code Reproduction

Example using React:

import { ScaleNotificationMessageCustomEvent } from "@telekom/scale-components";
import { ScaleModal, ScaleNotificationMessage } from "@telekom/scale-components-react";
import React, { useState } from "react";

const TestComponent: React.FC = () => {
  const [modalOpened, setModalOpened] = useState(false);
  const [notificationOpened, setNotificationOpened] = useState(false);
  const handleModalClose = () => {
    // This function is called as well when notification is dismissed!
    setModalOpened(false);
  };
  const handleNotificationClose = (event: ScaleNotificationMessageCustomEvent<void>) => {
    setNotificationOpened(false);
  };
  return (
    <>
      <button onClick={() => setModalOpened(true)}>Open Modal</button>
      <ScaleModal heading="Test Modal" opened={modalOpened} onScale-close={handleModalClose}>
        <div>
          <button onClick={() => setNotificationOpened(true)}>Open Notification</button>
        </div>
        <ScaleNotificationMessage opened={notificationOpened} dismissible onScale-close={handleNotificationClose}>
          <span slot="text">Test notification</span>
        </ScaleNotificationMessage>
      </ScaleModal>
    </>
  );
};

Workaround

We can prevent this behavior by stopping the scale-close event from bubbling inside the close event handler of the notification:

const handleNotificationClose = (event: ScaleNotificationMessageCustomEvent<void>) => {
  setNotificationOpened(false);
  event.stopPropagation(); // This prevents modal from closing.
};

Additional context

I assume that this was introduced with https://github.com/telekom/scale/issues/1057 by using the exact same event type scale-close. The custom event has bubbles: true and composed: true and therefore travels up the DOM tree. This is alright and should not be changed in my opinion.

However, the behavior is unexpected. Two ideas come to mind:

  1. Accept it as "works-as-designed" but describe this scenario and workaround in documentation of modal and notification, so that developers do not spend hours figuring out what is going on.
  2. Rename custom event to something more specific to notification, e.g. scale-notification-close - it would not trigger onScale-close handler of the modal anymore.

Hello there,

cannot agree more, scale-close is why to broad, used by modal, flyout, tags, notification, ...

stumbled upon this 7 month ago, for anyone who needs an fix:

A wrapper component which stops propagation


export const StopScaleClosePropagation = ({ children }: WithChildren) => {
  const ref = useRef<HTMLDivElement>(null);
  useEffectOnce(() => {
    const fn = (e: Event) => e.stopPropagation();
    ref.current?.addEventListener('scale-close', fn);
    return () => ref.current?.removeEventListener('scale-close', fn);
  });
  return <div ref={ref}>{children}</div>;
};


With this you can wrap the child and it works.

Same for using anything, that emits scale-close.

christopherGdynia avatar Jul 18 '23 05:07 christopherGdynia

Accept it as "works-as-designed" but describe this scenario and workaround in documentation of modal and notification, so that developers do not spend hours figuring out what is going on.

I completely agree, I will bring this up internally so that we can add this to the docs asap

felix-ico avatar Sep 06 '23 15:09 felix-ico