search

trimble-oss / modus-web-components

This library provides Modus components as web components - reusable, encapsulated UI elements that are framework agnostic (can be implemented in any site).
https://modus-web-components.trimble.com/
MIT License
31 stars 65 forks source link

Modus Table: row selection issue when density changes. #1983

Open yohernandez opened 6 months ago

yohernandez commented 6 months ago

Prerequisites

Describe the issue

There is an issue with the row selection when the density property changes. Steps to reproduce:

  1. Select a few rows.
  2. Change the density value
  3. See the preselected rows are loaded
  4. Select a row.
  5. See the previous selected rows are selected again.

checked issue

Reduced test cases

No response

What operating system(s) are you seeing the problem on?

No response

What browser(s) are you seeing the problem on?

No response

What is the issue regarding ?

@trimble-oss/modus-web-components

What version of npm package are you using ?

No response

Priority

Medium

What product/project are you using Modus Components for ?

e-builder

What is your team/division name ?

e-builder

Are you willing to contribute ?

Yes

Are you using Modus Web Components in production ?

No response

github-actions[bot] commented 6 months ago

Hello @yohernandez! Thanks for opening an issue. The Modus core team will get back to you soon (usually within 24-hours) and provide guidance on how to proceed. Contributors are welcome to participate in the discussion and provide their input on how to best solve the issue, and even submit a PR if they want to.

Please wait until the issue is ready to be worked on before submitting a PR, or you can reach out to the core team if it is time bound. For trivial things, or bugs that don't change the expected behaviors and UI, you can go ahead and make a PR.

apaddock commented 6 months ago

Also, the pagination change event is being triggered by the change to density. Should we trigger a density change event?

apaddock commented 4 months ago

@cjwinsor any advice on this one? Should this be fixed?

cjwinsor commented 3 months ago

@apaddock This is an issue and should be fixed, it most likely is related to state management between the tanstack table and our internal state handling, assuming we are managing that state. If not that we will need to dig a bit deeper.

prashanthr6383 commented 1 month ago

@apaddock This is an issue and should be fixed, it most likely is related to state management between the tanstack table and our internal state handling, assuming we are managing that state. If not that we will need to dig a bit deeper.

https://github.com/trimble-oss/modus-web-components/assets/168108000/70f29dc0-b241-46fb-958f-b7b5dc3e2b13

@cjwinsor The problem seems to be with the storybook. I have tested it in my local and it seems to be working fine.  The issue occurs not only with density changes but with all control items.

cjwinsor commented 1 month ago

@yohernandez Are you able to reproduce outside of storybook, or was this only identified as an issue in storybook?

yohernandez commented 1 month ago

@cjwinsor No, I am not. It's probably an issue in Storybook.

prashanthr6383 commented 1 month ago

@cjwinsor In the modus table component, a watch decorator is used for rowSelectionOptions which is causing this problem. To resolve this, we can leave the preSelected row as [] in the storybook or we can remove this feature ?

Image

modus-table.tsx

cjwinsor commented 1 month ago

@prashanthr6383 Can you explain more about leaving preSelected row as []? Do you mean how its set in the storybook?

prashanthr6383 commented 2 weeks ago

@cjwinsor Yes.  Whenever a property gets changed, the storybook updates all the properties instead of just the ones that we need. This is why when we change density or other properties in the storybook , it updates the property and sets the pre defined values for the remaining properties.

Do we need this preSelected row ? can we remove the preselected value if it is not necessary ?