SAP / ui5-webcomponents

UI5 Web Components - the enterprise-flavored sugar on top of native APIs! Build SAP Fiori user interfaces with the technology of your choice.
https://sap.github.io/ui5-webcomponents/
Apache License 2.0
1.47k stars 254 forks source link

[config][animationMode]: setAnimationMode() does not work #8964

Closed dreynol closed 1 month ago

dreynol commented 1 month ago

Bug Description

It is impossible to use setAnimationMode() to set the current animation to a valid mode.

Affected Component

Any component with animations (ui5-progress-indicator, ui5-panel, ui5-carousel, etc.)

Expected Behaviour

It is expected that calling setAnimationMode(mode), where mode is either a valid member of the AnimationMode enum/type (AnimationMode.Full, AnimationMode.Basic, AnimationMode.Minimal, or AnimationMode.None) or the corresponding string value ("full", "basic", "minimal", or "none"), would result in the current animation mode being changed to that value. This in turn is expected to modify the animation behavior of components that use animations. Specifically, setting the mode to "none" is expected to disable animations. This, however, is not the case.

// by default animationMode is initialized to "full"

setAnimationMode(AnimationMode.None);
console.log(getAnimationMode());
// expected output: "none"
// actual output: "full"

setAnimationMode("none"):
console.log(getAnimationMode());
// expected output: "none"
// actual output: "full"

setAnimationMode("None");
console.log(getAnimationMode());
// expected output: "full" (since the string "None" is not a valid type or value of AnimationMode)
// actual output: "None"

In the last example, while setAnimationMode("None") does change the currently set animation mode, it sets it to an invalid value so animations will not be disabled.

Isolated Example

https://stackblitz.com/edit/js-32lj5a?file=index.js,index.html

Steps to Reproduce

Try to use setAnimationMode(AnimationMode.None) or setAnimationMode("none") to disable animations for a component (e.g. a ui5-progress-indicator) and observe that animations still occur when the state of the component changes. (See "Expected Behaviour" above or the stackblitz link for more detail.)

Log Output, Stack Trace or Screenshots

No response

Priority

High

UI5 Web Components Version

1.24.3

Browser

Chrome

Operating System

No response

Additional Context

No response

Organization

No response

Declaration

dreynol commented 1 month ago

In this code from packages/base/src/config/AnimationMode.ts

const setAnimationMode = (animationMode: `${AnimationMode}`) => {
    if (animationMode in AnimationMode) {
        curAnimationMode = animationMode;
    }
};

the "in" operator in the if statement checks if the passed-in animationMode is a property in the AnimationMode enum, not if it is a value in the enum, as seems to be intended. So given the following enum,

enum AnimationMode {
    Full = "full",
    Basic = "basic",
    Minimal = "minimal",
    None = "none"
}

the only values of animationMode that will make the if-statement evaluate to true are "Full", "Basic", "Minimal", or "None" (capitalized as the AnimationMode keys are), which means setAnimationMode() can only be used to set curAnimationMode to one of those capitalized values. So setAnimationMode("None") will result in curAnimationMode = "None", but when determining whether to use animation, components like the ui5-progress-indicator compare the currently set mode to the lowercase value:

// packages/main/src/ProgressIndicator.ts
class ProgressIndicator extends UI5Element {
    [...]

    get shouldAnimate() {
        return getAnimationMode() !== AnimationMode.None;
    }

    [...]
}

That comparison will be "None" !== "none". So no matter what is passed to setAnimationMode(), the shouldAnimate() method will always return true.

GerganaKremenska commented 1 month ago

Hello colleagues,

Can you take over the issue regarding setAnimationMode(), it seems there is an issue with it. It is reproducible in the stackblitz.

Best Regards, Gergana