h2oai / wave

Realtime Web Apps and Dashboards for Python and R
https://wave.h2o.ai
Apache License 2.0
3.9k stars 323 forks source link

Table filter not working when cell value is a substring of another cell value #2141

Open marek-mihok opened 9 months ago

marek-mihok commented 9 months ago

Wave SDK Version, OS

Wave 0.26.0, MacOS 13.5.1, Chrome Version 117.0.5938.88 (Official Build) (arm64)

Actual behavior

I am unable to filter out rows by a cell value if its value is a substring of another cell value.

https://github.com/h2oai/wave/assets/23740173/fceb80db-02b2-4c5f-90ad-7b4b620262e1

Expected behavior

To be able to filter out rows by a cell value if its value is a substring of another cell value.

Steps To Reproduce

from h2o_wave import main, app, Q, ui

@app('/demo')
async def serve(q: Q):
    q.page['form'] = ui.form_card(box='1 1 3 3', items=[
        ui.table(
            name='issues',
            columns=[ui.table_column(name='text', label='First name', filterable=True)],
            rows=[
                    ui.table_row(name='name1', cells=['Joseph']),
                    ui.table_row(name='name2', cells=['Josephine']),
            ],
        )
    ])
    await q.page.save()

Issue is probably in this line with using of include() - e.g. 'Josephine'.includes(name) returns true regardless of name being Joseph or Josephine.

YounesDjelloul commented 9 months ago

Hi Marek. This is my first time here and would like to create a PR for this bug as I successfully solved it. Basically the problem exactly is in the table.tsx file in the filter function. It used to be using String(somedata).includes(somefilter). However this would produce this bug. We just need to replace this line with: String(item[filterKey]) === filterVal.

And the issue would be solved then. That's all. Lemme know please how I could contribute by opening a new PR because I keep getting 403 :')

marek-mihok commented 9 months ago

Hi @YounesDjelloul, thanks! 🙂

We just need to replace this line with: String(item[filterKey]) === filterVal

This resolves the issue, but looks like it introduces another one since it causes a failing test case. Probably there is a reason why includes() was used instead of equality check ===. Can you please make sure all tests are passing?

Lemme know please how I could contribute by opening a new PR because I keep getting 403

You need to make a PR from a forked repo.

YounesDjelloul commented 9 months ago

Thank you Marek for pointing to this!

However I still can't understand the role of this test exactly! It's creating rows with cells values as follows: "TAG1", "TAG2,TAG3", "TAG2". And after that it's trying to filter the rows by TAG1 and TAG3 which is impossible because the cell values actually doesn't have "TAG3" as plain, it has "TAG2,TAG3". So in other words the available filters that would be available in this testcare are: "TAG1", "TAG2", "TAG2, TAG3". Which would be shown for a normal user if we try to simulate the same case!

Am I missing something here?