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

VariantManagement: impossible to trigger delete action for public (global) views #6706

Open
1 task done
superdyzio opened this issue Dec 6, 2024 · 2 comments
Open
1 task done
Labels
author action bug Something isn't working SAP Fioneer

Comments

@superdyzio
Copy link
Contributor

superdyzio commented Dec 6, 2024

Describe the bug

There is no possibility to force delete icon to show next to the public (global: true variants). I was expecting hideDelete={false} to do the trick and according to the logic in ManageViewsTableRows.tsx:216 it should:

<TableCell>
        {!(hideDelete ?? global) && (
          <Button
            tooltip={a11yDeleteText}
            accessibleName={a11yDeleteText}
            icon={declineIcon}
            design={ButtonDesign.Transparent}
            onClick={handleDelete}
            data-children={children}
          />
        )}
      </TableCell>

but in our case it is not enough. All of my elements have data-hide-delete="false" attribute but hideDelete is still undefined. Do I need to provide some additional data?

Unfortunately it works correctly in the Stackblitz example 😅 so it's possible that the issue is actually on our side, but I cannot track it down and since I can see the attributes correctly added to the elements I have a feeling that something might be broken also on the library side

I'm looking forward to explanation of the issue and if needed, as usual, I'll be happy to provide a PR.

Isolated Example

https://stackblitz.com/edit/github-cbs6vz?file=src%2FApp.tsx

Reproduction steps

  1. Have some public and some private views.
  2. Set hideDelete={false} on all <VariantItem> instances.
  3. Open manage views dialog.
    ❌ icon is visible only next to the private (global: false) views.

Expected Behaviour

When I set hideDelete={false} I get the delete variant icon no matter what. I expect boolean flags to be independent and predictable, if I'd like to hide delete button for public variants only I would do hideDelete={item.global}.

Screenshots or Videos

image
(apply automatically header visible in the screenshot is a different bug, tracked here: #6618)

UI5 Web Components for React Version

2.4.0

UI5 Web Components Version

2.4.0

Browser

Chrome

Operating System

MacOS Sequoia 15.1.1

Additional Context

Our usage of VariantItem:

<VariantItem
    favorite={favorite}
    labelReadOnly={labelReadOnly}
    isDefault={isDefault}
    global={global}
    hideDelete={false}
    // todo: we might be able to remove condition when https://github.com/SAP/ui5-webcomponents-react/issues/6617 is resolved
    {...(readOnly === true
      ? {
          readOnly,
        }
      : {})}
    selected={selected}
  >
    {children}
  </VariantItem>

Relevant log output

No response

Organization

SAP Fioneer

Declaration

  • I’m not disclosing any internal or sensitive information.
@Lukas742
Copy link
Contributor

Lukas742 commented Dec 9, 2024

Hi @superdyzio

unfortunately, we can't assist you with this issue without a reproducible example. I tested the prop with both allowed values (false, true) and even with undefined (which only won't throw a TypeScript error if exactOptionalPropertyTypes is not enabled) and it works as intended for each value.

In v2.5.0, we addressed several issues related to boolean handling in the component, so updating might resolve this issue as well. If it doesn’t, please try to isolate the problem. If that's not feasible, feel free to contact me internally, and we can debug the component together.

@superdyzio
Copy link
Contributor Author

thank you very much for the time spent on that @Lukas742! 🙏
we were playing around with our code this morning and we noticed that it works correctly when we remove our wrapper (UiVariantItem) - it is only passing props (we have a convention with wrappers, because sometimes we need to align something), but still it somehow breaks the functionality

do you have any ideas why? what to look for?
is there any logic that relies on VariantItem being direct children of VariantManagement? I didn't see it and I also tried adding wrapper component to the stackblitz example, but it's working correctly there

for now it's fixed by just removing the wrapper, and since I cannot reproduce it outside of our code, I'm perfectly fine with this issue being closed, I'm just hoping that your answers to above questions will give me some new leads 😄

it's awesome that you just released v2.5.0, it will resolve some todos on our side, thanks! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author action bug Something isn't working SAP Fioneer
Projects
Status: 🆕 New
Development

No branches or pull requests

2 participants