spectrum-web-components icon indicating copy to clipboard operation
spectrum-web-components copied to clipboard

[Bug]: _$rowType$: 1 doesn't produce a row without a checkbox

Open Westbrook opened this issue 2 years ago • 5 comments

Code of conduct

  • [X] I agree to follow this project's code of conduct.

Impacted component(s)

Table

Expected behavior

Adding an item with {_$rowType$: 1} should produce a row without a checkbox.

Actual behavior

A checkbox is present?

Screenshots

No response

What browsers are you seeing the problem in?

No response

How can we reproduce this issue?

No response

Sample code that illustrates the problem

No response

Logs taken while reproducing problem

No response

Westbrook avatar Aug 04 '22 16:08 Westbrook

@Westbrook to make sure I understand this issue - is this item an example of one that should not have a checkbox?

https://studio.webcomponents.dev/edit/ErIkLgkFbEuRoUImMZVp/src/index.ts?p=stories

hunterloftis avatar Aug 04 '22 18:08 hunterloftis

The demo was caught a couple of versions back, try this one: https://studio.webcomponents.dev/edit/XZEYXGb1cWTttk2S889L/src/index.ts?p=stories

Westbrook avatar Aug 04 '22 19:08 Westbrook

Based on this example, it looks like either this issue was resolved between those versions, or the specific trigger that creates an unwanted checkbox is something other than what's in this description.

@Westbrook can you point me to someone who can reproduce this issue, since I can't?

hunterloftis avatar Aug 04 '22 19:08 hunterloftis

We are passing an asset type to the table row for normal row and null for load more row we cannot set _$rowType$: 1 in null. All the workarounds that we have thought will require huge changes on organizer level (e.g. changing asset type everywhere). Better solution would be to add an attribute like hideCheckboxAtRowIndex=${[index]} in <sp-table> which can be used to hide checkboxes at particular indexes.

harprees avatar Aug 06 '22 15:08 harprees

We don't think that hideCheckboxAtRowIndex is an appropriate API for this interaction. Leveraging _$rowType$: 1 give a lot more flexibility; both in it being able to support more than one of them at a time and in the way you could carry data required to render that row in them in a way that null cannot. However, if you know the index, you should be able to easily replace your null entry with the correct data shape via: items.splice(index, 1, { _$rowType$: 1 }) without needing to address any issues you may be encountering interacting with this data further up in your application.

Beyond that, while we have no use case now, the benefits of the _$rowType$ API is that we would be able to add other non-default row types in the future, so ensuring that you application can natively handle this API early would likely benefit you and your team in the future.

Westbrook avatar Aug 08 '22 12:08 Westbrook

@Westbrook , I was able to use _$rowType$ in our application. It works fine. We can close this ticket.

harprees avatar Aug 13 '22 16:08 harprees

Great! Let us know how else we can support your release in this area.

Westbrook avatar Aug 13 '22 17:08 Westbrook