CarlosNZ / json-edit-react

React component for editing/viewing JSON/object data
https://carlosnz.github.io/json-edit-react/
MIT License
187 stars 18 forks source link

"showCollectionCount" prop, doesn't update to filtered data #113

Open MattChowski opened 2 months ago

MattChowski commented 2 months ago

When using "searchText" and "searchFilter" with "showCollectionCount" prop, the collection count doesn't update to the filtered options, resulting in a confusing UX for end users

image

You can see in the image that action_result has 5 items whereas the JSON shows only one, data because I filtered for it.

As a workaround I could probably filter the json on the outside and pass it as a new prop to the JSON viewer but that would re-render more times unnecessarily. I was hoping to use the internal searching functions.

CarlosNZ commented 2 months ago

Yeah, that's a good point, although I'm not totally sure what the "correct" solution would be. Strictly speaking, the search filter is only supposed to be a "display" thing, we're not trying to say that the data isn't there any more, and I think it would be misleading (in some cases at least) to say otherwise.

As a general rule, I would say that if you just want to temporarily hide values for convenience, and the user knows about it, then the current behaviour is correct. However, if you want to truly hide the data and present it to the user as though it was never there, then it's probably best to filter from the outside before passing into the component.

Having said that, I'm not totally sold on this distinction and I can think of cases where the opposite is true in both directions. So I'm open to further discussion.

It would be a reasonable refactor to be able to show the filtered item count (especially to do it in a performant way), so I'd like to get 100% clear on what the preferred/expected behaviour should be before changing anything. At the very least, I think it'd need to be an optional setting, so you could display either the filtered count or the total count (whichever makes most sense for your use case).

In the meantime, you can either: