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

isStuckToBottom behaviour when filtering #1477

Closed
dsmmcken opened this issue Sep 1, 2023 · 1 comment · Fixed by #1579, deephaven/deephaven-core#4736 or #1616
Closed

isStuckToBottom behaviour when filtering #1477

dsmmcken opened this issue Sep 1, 2023 · 1 comment · Fixed by #1579, deephaven/deephaven-core#4736 or #1616
Assignees
Labels
bug Something isn't working

Comments

@dsmmcken
Copy link
Contributor

dsmmcken commented Sep 1, 2023

Description

Given a table and a scroll position at the top of the table, filtering to a result of less than a full viewport, then clearing the filter sets the table to isStuckToBottom (and further, only applies after a re-render). As a user this can be confusing and frustrating that you end up at the bottom of a table after the filter clears.

Although, this may be the desired behaviour if the user was already at the bottom. Open to heuristic suggestions for this behaviour. Maybe isStuckToBottom isn't triggered until on short viewports until it a tick actually goes past the end of the screen.

In comparison, if you are at the top, filter by a result that has a full viewport worth of results (example filter MyBoolean = true from below), then clear that filter you remain at the top. So behaviour is inconsistent.

Steps to reproduce

  1. Create a simple table
from deephaven import empty_table

simple_static = empty_table(1000).update([
        "MyString=new String(`a`+(char) ((i%3)+97))",
        "MyInt=new Integer(i)",
        "MyLong=new Long(i % 3)",
        "MyDouble=new Double(i+i/10)",
        "MyFloat=new Float(i+i/10)",
        "MyBoolean=new Boolean(i%2==0)",
        "MyChar= new Character((char) ((i%26)+97))",
        "MyShort=new Short(Integer.toString(i%32767))",
        "MyByte= new java.lang.Byte(Integer.toString(i%127))"])
  1. From a scroll position at the top of the table, filter the table to 1 result (filter by MyInt = 0)
  2. Clear the filter with a shortcut (ctrl/cmd+e)
  3. Move the mouse anywhere over the table to trigger a re-render

Expected results

I feel like I would expect the table to be back at the top.

Actual results

The table is stuck to the bottom. (It doesn't move until after step 4, which is even weirder).

Versions

Engine Version: 0.27.0
Web UI Version: 0.46.0
Java Version: 17.0.7
Barrage Version: 0.6.0

@dsmmcken dsmmcken added bug Something isn't working triage Issue requires triage labels Sep 1, 2023
@vbabich vbabich added this to the Backlog milestone Sep 6, 2023
@vbabich vbabich removed the triage Issue requires triage label Sep 6, 2023
@ethanalvizo ethanalvizo self-assigned this Oct 13, 2023
@mofojed mofojed modified the milestones: Backlog, October 2023 Oct 17, 2023
ethanalvizo added a commit that referenced this issue Oct 27, 2023
Closes #1477 

Previous comments are in #1571 PR. 

Note: Once this PR is merged #1571 can go in as well
mofojed pushed a commit to deephaven/deephaven-core that referenced this issue Oct 30, 2023
Release notes https://github.com/deephaven/web-client-ui/releases/tag/v0.52.0

# [0.52.0](deephaven/web-client-ui@v0.51.0...v0.52.0) (2023-10-27)


### Bug Fixes

* stuck to bottom on filter clear ([#1579](deephaven/web-client-ui#1579)) ([ef52749](deephaven/web-client-ui@ef52749)), closes [#1477](deephaven/web-client-ui#1477) [#1571](deephaven/web-client-ui#1571) [#1571](deephaven/web-client-ui#1571)
* Theming - switched from ?inline to ?raw css imports ([#1600](deephaven/web-client-ui#1600)) ([f6d0874](deephaven/web-client-ui@f6d0874)), closes [#1599](deephaven/web-client-ui#1599)


### BREAKING CHANGES

* Theme css imports were switched from `?inline` to
`?raw`. Not likely that we have any consumers yet, but this would impact
webpack config.

Co-authored-by: deephaven-internal <deephaven-internal@users.noreply.github.com>
@mattrunyon mattrunyon reopened this Nov 2, 2023
@mattrunyon
Copy link
Collaborator

mattrunyon commented Nov 2, 2023

The original fix for this made it impossible to get unstuck from the bottom. See #1615. Reverted in #1616

The issue in the original fix was setting a shouldStickBottom based on the wheel event. There are other ways to scroll (arrow keys, grab and drag the scroll thumb) which are unable to update the sticky property. Also, scrolling up w/ wheel has a negative delta and was unable to update the sticky property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment