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

Implement multi-column row keys for ui.table #4105

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

me21
Copy link
Contributor

@me21 me21 commented Dec 16, 2024

It is now possible to specify a list of keys as row-key property. Example:

summary_table = ui.table(
    columns=summary_columns,
    rows=summary_rows,
    row_key=['date', 'name']
    pagination=10,
    selection='single'  # Enable single row selection
)

In the current version, it's possible to work with multiple columns as composite table key as:

ui.table(...).props(':row-key="row => row.date+row.name"')

but it doesn't handle deselection properly, as row_key is still equal to 'id' if not specified in the arguments list...

It is now possible to specify a list of keys as row-key property.
@me21 me21 changed the title Implement multi-column row keys Implement multi-column row keys for ui.table Dec 16, 2024
Because otherwise if row_key is changed later through setter, handle_selection() would still use the outdated value from __init__().
@me21
Copy link
Contributor Author

me21 commented Dec 16, 2024

This will still fail, if concatenated cell values are the same for different rows, e.g. ('a', 'b') and ('ab', ''). But this is already an improvement, please feel free to build on this.
I'm also not sure if it's possible to throw out self._row_key attribute and use self._props['row-key'] instead, as it's referenced as row-key in one place and :row-key in another -- are these different props from Python point of view or not?

@falkoschindler
Copy link
Contributor

Thanks for this pull request, @me21!

How should summary_rows and summary_columns look like for the new API to work?

are these different props from Python point of view or not?

In Python self._props['row-key'] and self._props[':row-key'] are different dictionary items. In JavaScript props[':row-key'] will be evaluated as JavaScript code and the result will be written into props['row-key']. So if a "dynamic" prop with leading colon : is given, it overwrites its "normal" couterpart.

@me21
Copy link
Contributor Author

me21 commented Dec 16, 2024

Summary_columns and summary_rows are just my variable names copied from my project. The meaning of columns and rows parameters is unchanged.

The code assumes there are columns named "date" and "name". And their values are strings.

In Python self._props['row-key'] and self._props[':row-key'] are different dictionary items.

Ok, then I guess we'll need an object attribute to store the user assigned value to the row_key property... It's already done in the PR.

@me21
Copy link
Contributor Author

me21 commented Dec 16, 2024

Now I think we could convert the row_key argument of ui.table to list unconditionally and avoid type checking in handle_selection... just an optimization.

@falkoschindler
Copy link
Contributor

Summary_columns and summary_rows are just my variable names copied from my project.

I was wondering about their content in order to understand the problem with the existing implementation. But I got it now: Some of QTable's internals require a unique row key. In the following example, a single "Alice" can't be selected because the key value of both "Alice" rows is identical.

columns = [
    {'name': 'name', 'label': 'Name', 'field': 'name'},
    {'name': 'age', 'label': 'Age', 'field': 'age'},
]
rows = [
    {'name': 'Alice', 'age': 18},
    {'name': 'Alice', 'age': 19},
    {'name': 'Bob', 'age': 21},
    {'name': 'Carol'},
]
ui.table(columns=columns, rows=rows, row_key='name', selection='multiple')

Combining multiple columns into a row key allows to keep them selectable and sortable despite the lack of a unique column.

I'll have to think about the current approach of simply string-concatenating columns since it breaks in edge cases like you described. And it will definitely break for completely identical rows. In this case the user needs to add unique IDs anyway.

@me21
Copy link
Contributor Author

me21 commented Dec 16, 2024

Sure. String-concatenating columns was my quick and dirty fix that worked for my use case. To make it more foolproof, it's possible to string-concatenate them with a separator that is unlikely to be contained in cell values (some non-printable character? \u0000?).
And of course we can't distinguish fully identical rows. I'm not sure NiceGUI should add an extra hidden field behind the scenes in this case, though it's possible.

@falkoschindler
Copy link
Contributor

@me21 After thinking about it once again, we're still sceptical about this API change. If users are aware of the problem and would benefit from multi-column row keys, they have already two options:

  1. Define a custom row-key function like .props(':row-key="row => row.date+row.name"'). This can be fine-tuned to the specific application and doesn't depend on a pre-determined separator.
  2. Add a unique attribute to the row dictionaries. In situations like the name/age table, this would be the preferred method since combining name and age will never guarantee uniqueness.

We would suggest to add two new demos to the table documentation, one for each of these two solutions. We hope that this helps the users to find the right approach for their application.

@me21
Copy link
Contributor Author

me21 commented Dec 18, 2024

Unfortunately, the first solution has issues with row deselection, as the code that handles deselection doesn't play well with row-key as a function. So the only solution would be to add hidden unique attribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants