Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

VariantManagament: Missing button to save changes to already saved view #6617

Closed
1 task done
superdyzio opened this issue Nov 13, 2024 · 4 comments · Fixed by #6628
Closed
1 task done

VariantManagament: Missing button to save changes to already saved view #6617

superdyzio opened this issue Nov 13, 2024 · 4 comments · Fixed by #6628

Comments

@superdyzio
Copy link
Contributor

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.
@Lukas742
Copy link
Contributor

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.

@superdyzio
Copy link
Contributor Author

superdyzio commented Nov 17, 2024

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?

@Lukas742
Copy link
Contributor

@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 :)

@ui5-webcomponents-react-bot
Copy link
Contributor

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

The release is available on v2.5.0

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🆕 New
Development

Successfully merging a pull request may close this issue.

3 participants