SAP/ui5-webcomponents-react

VariantManagament: Missing button to save changes to already saved view

Closed this issue · 4 comments

Describe the bug

It looks like in the variant manager, we don’t have the option to save changes to the already saved view. At the moment, after introducing changes, we can only save it as a new view.
It works correctly from time to time, but there is no clear rule.
The data-readonly attribute was not present on the item and still there was no Save button. We also tried completely removing readOnly property from the variant object passed to the component and still no Save button.

Isolated Example

No response

Reproduction steps

  1. Create new variant that is not readonly
  2. Change something in the config
  3. Try to save (missing Save button)

Expected Behaviour

Save button is visible when a variant that is not readonly was modified.

Screenshots or Videos

image

UI5 Web Components for React Version

2.1.1

UI5 Web Components Version

2.2.0

Browser

Chrome

Operating System

MacOS Sequoia 15.1

Additional Context

Analytical tables used with the VariantManagament are very big, up to almost 300 columns and god-knows-how-many rows.
We keep our variants in the local storage, including readOnly field and we confirmed that it is not getting any default value when retrieved.

Relevant log output

No response

Organization

SAP Fioneer

Declaration

  • I’m not disclosing any internal or sensitive information.

Hi @superdyzio

I wasn't able to reproduce this issue, the save button is displayed for me if the VariantManagement is in "dirty" state (except for items with readOnly). Please add an isolated example preferably by leveraging StackBlitz. You can either use the example I've created when trying to reproduce the issue, or our template.

hi @Lukas742, thanks for the effort!
in this case it is actually on our side - we had one place adding readOnly: undefined and since the VariantManagement is checking for existence of readOnly property it was not showing the Save button

however, I'd still suggest aligning this logic to be more consistent, because right now I can have readOnly: false and end up with Variant treated as read only - I'll be happy to fix it

// todo: this applies if `readOnly` is set to `false` as well since the value is read via data-attribute and is therefore a string not a boolean
// this todo would be resolved after the change
  const showSaveBtn = dirtyState && selectedVariant && !selectedVariant.readOnly;

btw what's the reasoning behind current hasOwnProperty approach?

@superdyzio you're right, I also noticed that when looking into the behavior of the other issue you raised. The PR you've opened will fix this as well. Thanks again :)

🎉 This issue has been resolved in version v2.5.0 🎉

The release is available on v2.5.0

Your semantic-release bot 📦🚀