fix(ui5-illustrated-message): prevent infinite resize

[plamenivanov91]

By caching the offsetWidth of the IllustratedMessage for each media, in the case of infinite resize, we check if the newly set media isn't with the same offsetWidth as the last time. If it is (case of infinite resize), we don't change the media.
Fixes: #5852
Thank you for your contribution! 👏

To get it merged faster, kindly review the checklist below:

Pull Request Checklist

• Reviewed the Contributing Guidelines
  • Especially the How to Contribute section
• Correct commit message style
  • Bugfixes should generally include a test to cover the issue.

[ilhan007]

It looks good, there are some inline comments, that you can find.
In addition to the comments, could you add some basic information in the commit msg body - what is this about and how the bug is now resolved.

[dobrinyonkov]

We could also fix this by using throttle.

const throttle = (fn, delay) => {
    let lastCall = 0;
    return function (...args) {
        // use performance.now() instead of Date.now() for better precision
        const now = performance.now();
        if (now - lastCall < delay) {
            return;
        }
        lastCall = now;
        return fn(...args);
    };
};

IllustratedMessage.js

   handleResize = throttle(() => {
        if (this.size !== IllustrationMessageSize.Auto) {
            return;
        }

        if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.BASE) {
            this.media = IllustratedMessage.MEDIA.BASE;
        } else if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.SPOT) {
            this.media = IllustratedMessage.MEDIA.SPOT;
        } else if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.DIALOG) {
            this.media = IllustratedMessage.MEDIA.DIALOG;
        } else {
            this.media = IllustratedMessage.MEDIA.SCENE;
        }
    }, 300)

[plamenivanov91]

We could also fix this by using throttle.

const throttle = (fn, delay) => {
    let lastCall = 0;
    return function (...args) {
        // use performance.now() instead of Date.now() for better precision
        const now = performance.now();
        if (now - lastCall < delay) {
            return;
        }
        lastCall = now;
        return fn(...args);
    };
};

IllustratedMessage.js

   handleResize = throttle(() => {
        if (this.size !== IllustrationMessageSize.Auto) {
            return;
        }

        if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.BASE) {
            this.media = IllustratedMessage.MEDIA.BASE;
        } else if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.SPOT) {
            this.media = IllustratedMessage.MEDIA.SPOT;
        } else if (this.offsetWidth <= IllustratedMessage.BREAKPOINTS.DIALOG) {
            this.media = IllustratedMessage.MEDIA.DIALOG;
        } else {
            this.media = IllustratedMessage.MEDIA.SCENE;
        }
    }, 300)

Although this solution is a lot better performance wise, it introduces two new unwanted behaviors.

1. If you expand the Dialog with your cursor slowly, we go to the new breakpoint, then the old one (while still expanding). We end up with the bigger one (correct), but I feel like this might be reported as bug.
2. If you expand it to Dialog breakpoint and then quickly shrink it to go to Spot, we end up with Dialog (incorrect). The chances of this happening might be reduced by making the delay lower, but the chance is still there. By the way if make the delay a lot lower (10ms for example), the initial bug is present (infinite resize).

Let me know what you think about my findings and if you think that the throttle solution should take place. Or if you have any other ideas.

[plamenivanov91]

In the end me and @dobrinyonkov decided to use offsetWidth instead of boundingClientRect. Although this solution introduces slightly more flickering when resizing in the problematic zone, it's a lot better performance wise.
I decided to leave the shallowEqual comparison util in case of future needs.

[dobrinyonkov]

Looks good to me! I only suggest to remove the shallowEqual util to follow the YAGNI principle.

outdated