fix: DH-22976 ui.table filters to update when changed programmatically#1377
fix: DH-22976 ui.table filters to update when changed programmatically#1377SimonVutov wants to merge 6 commits into
Conversation
|
ui docs preview (Available for 14 days) |
SimonVutov
force-pushed
the
feat-DH-21841-fix-user-to-modify
branch
from
2e9c1ae to
8961441
Compare
|
ui docs preview (Available for 14 days) |
SimonVutov
force-pushed
the
feat-DH-21841-fix-user-to-modify
branch
from
8961441 to
8eba401
Compare
|
ui docs preview (Available for 14 days) |
mofojed
left a comment
mofojed
left a comment
There was a problem hiding this comment.
This does allow the user to change the sorts, which is great. However, those changed sorts do not persist as they should (as described in my comment on #1358 (comment)).
I think to fix this properly (for both this case and the filters case described in DH-22976 that I've also assigned to you), we'll need changes in IrisGrid to handle this correctly. I think. I have a snippet specifically for sorting on #1358 (review)
|
ui docs preview (Available for 14 days) |
SimonVutov
changed the title
|
ui docs preview (Available for 14 days) |
1 similar comment
|
ui docs preview (Available for 14 days) |
There was a problem hiding this comment.
Add e2e test for this guy to make sure of the following:
- Filters/sorts update correctly when changed programmatically
- User can still change the filters and sorts, and those changes persist after refreshing
See an example e2e test with persistence https://github.com/deephaven/deephaven-plugins/pull/1379/changes#diff-297dc965f80789b8ed682341089a82d6f946c164dcab51bb3b08b34dff151a02
|
ui docs preview (Available for 14 days) |
|
Had to re-run the e2e tests a few times. Summary:
|
There was a problem hiding this comment.
This doesn't seem to work correctly for filtering, using the snippet from the ticket:
from deephaven import ui
import deephaven.plot.express as dx
_stocks = dx.data.stocks()
t = _stocks.where("Sym = `CAT`") # Applied when query is run
@ui.component
def table_filter_picker():
exchange, set_exchange = ui.use_state("NYPE")
return ui.flex(
ui.picker("NYPE", "PETX", on_selection_change=set_exchange, selected_key=exchange, label="Exchange"),
ui.heading(exchange),
ui.table( # Filters applied when table is opened on the client
_stocks,
show_quick_filters=True,
quick_filters={
"Sym": "CAT",
"Exchange": exchange,
"Price": ">=100"
}
),
direction="column"
)
t2 = table_filter_picker()Changing the dropdown does not update the filters.
EDIT: I see I probably need deephaven/web-client-ui#2712 as well. Put that in the PR description so it's clear; we shouldn't merge this until that fix goes in.
| // re-applied whenever their value changes. We stabilize them by content so an | ||
| // unrelated re-render (new reference, identical content) does not re-apply and | ||
| // replace changes the user made in the UI. | ||
| const stableSorts = useIsEqualMemo(sorts, deepEqual); |
There was a problem hiding this comment.
This shouldn't be necessary - we use patching in the deephaven.ui render so it should be a stable object unless there's actually changes. If that's not the case, there's something else that needs to be fixed in the renderer.
We added patching in: #1313
|
ui docs preview (Available for 14 days) |
2 participants
Note: Merge deephaven/web-client-ui#2712 first, then merge this PR.
sorts / quick_filters are server-owned: the server controls the value, so updating the prop re-applies it and replaces whatever the user changed in the UI.
New default_sorts / default_quick_filters are user-owned: they set the initial value, then the user owns it from there — their changes are persisted and restored on reload. Use one or the other for a given facet, not both.