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.

Sign up for GitHub

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

Jump to bottom

Error when Edited Cell scrolls out of Viewport #2087

Closed
dgodinez-dh opened this issue Jun 20, 2024 · 1 comment · Fixed by #2148 or deephaven/deephaven-core#5790
Closed

Error when Edited Cell scrolls out of Viewport #2087

dgodinez-dh opened this issue Jun 20, 2024 · 1 comment · Fixed by #2148 or deephaven/deephaven-core#5790
Assignees
@AkshatJawne
Labels
bug Something isn't working
Milestone
July 2024

Comments

@dgodinez-dh
Copy link
Contributor

Description

When an Edited Cell is scrolled out the viewport, the table fails with an error "Unable to get ModelIndex for row N". This was discovered while investigating the Enterprise ticket DH-16515. I have only reproduced this scrolling rows, but I suspect it also happens with columns. Tested with both the Legacy and Core+ engines.

Steps to reproduce

  1. Create an InputTable.
  2. Double click on cell to start editing.
  3. While the cell is still in editing mode, scroll it off the screen.

Expected results

The table should scroll like normal.

Actual results

The displays an error "Unable to get ModelIndex for row N"

Additional details and attachments

image

Versions
This was found on Enterprise 1.20231218.146 with Core engine 0.33.4
@dgodinez-dh dgodinez-dh added bug Something isn't working triage Issue requires triage labels Jun 20, 2024
@dgodinez-dh
Copy link
Contributor Author

Meant to add:
In Grid.tsx in renderInputField we call getModelRow which throws an unhandled Error.
I suspect the same thing will happen in getModelColumn

@mofojed mofojed added this to the July 2024 milestone Jun 25, 2024
@mofojed mofojed removed the triage Issue requires triage label Jun 25, 2024
@AkshatJawne AkshatJawne mentioned this issue Jul 16, 2024
Merged
@mofojed mofojed mentioned this issue Jul 17, 2024
Merged
mofojed pushed a commit to deephaven/deephaven-core that referenced this issue Jul 17, 2024
@github-actions @deephaven-internal
feat: Update web version 0.86.0 (#5790)
314a30c 
## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.86.0

### Features

* Add option to disable WebGL rendering
([#2134](deephaven/web-client-ui#2134))
([011eb33](deephaven/web-client-ui@011eb33))
* Core plugins refactor, XComponent framework
([#2150](deephaven/web-client-ui#2150))
([2571fad](deephaven/web-client-ui@2571fad))
* IrisGridTheme iconSize
([#2123](deephaven/web-client-ui#2123))
([58ee88d](deephaven/web-client-ui@58ee88d)),
closes [#885](deephaven/web-client-ui#885)
* Partitioned Table UI Enhancements
([#2110](deephaven/web-client-ui#2110))
([de5ce40](deephaven/web-client-ui@de5ce40)),
closes [#2079](deephaven/web-client-ui#2079)
[#2066](deephaven/web-client-ui#2066)
[#2103](deephaven/web-client-ui#2103)
[#2104](deephaven/web-client-ui#2104)
[#2105](deephaven/web-client-ui#2105)
[#2106](deephaven/web-client-ui#2106)
[#2107](deephaven/web-client-ui#2107)
[#2108](deephaven/web-client-ui#2108)
[#2109](deephaven/web-client-ui#2109)
[#2049](deephaven/web-client-ui#2049)
[#2120](deephaven/web-client-ui#2120)
[#1904](deephaven/web-client-ui#1904)


### Bug Fixes

* error when edited cell is out of grid viewport
([#2148](deephaven/web-client-ui#2148))
([3fccd43](deephaven/web-client-ui@3fccd43)),
closes [#2087](deephaven/web-client-ui#2087)

## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.2


### Bug Fixes

* Fix missing scrim background on LoadingOverlay
([#2098](deephaven/web-client-ui#2098))
([c9ed895](deephaven/web-client-ui@c9ed895))

## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.1

##
[0.85.1](deephaven/web-client-ui@v0.85.0...v0.85.1)
(2024-07-08)


### Bug Fixes

* re-export remaining types needed by dh ui from @react-types/shared
([#2132](deephaven/web-client-ui#2132))
([2119a61](deephaven/web-client-ui@2119a61))



## Release notes
https://github.com/deephaven/web-client-ui/releases/tag/v0.85.0

### Features

* ComboBox - @deephaven/jsapi-components
([#2077](deephaven/web-client-ui#2077))
([115e057](deephaven/web-client-ui@115e057)),
closes [#2074](deephaven/web-client-ui#2074)


### Bug Fixes

* Allow ComboBox to accept the FocusableRef for ref
([#2121](deephaven/web-client-ui#2121))
([8fe9bad](deephaven/web-client-ui@8fe9bad))
* Ref was not being passed through for Picker
([#2122](deephaven/web-client-ui#2122))
([a11e2ce](deephaven/web-client-ui@a11e2ce))

Co-authored-by: deephaven-internal <66694643+deephaven-internal@users.noreply.github.com>
@mofojed mofojed mentioned this issue Jul 18, 2024
Merged
mofojed pushed a commit that referenced this issue Jul 23, 2024
@AkshatJawne @mofojed
fix: error when edited cell is out of grid viewport (#2148)
e9e49e6 
Resolves #2087 

**Steps Moving Forward:**
- Clarify whether we want to be able to **handle edits that are not in
the current viewport** -- with my understanding, this will mean we will
have to load in more grid data, which does not seem overly valuable (in
terms of the efficiency tradeoff) since we are dealing with an
improbable edge case where the user begins to edit in the viewport,
continues editing outside of the viewport where the cell originally was,
and then wants to commit those changes outside as well

** A better solution for the edge case above IMO is to **create a new
enhancement ticket** where we attempt to have the editing cell be
floating/sticky in the grid viewport, similar to how it is done on
Google Sheets **

**_Ticket Created:_**
#2153

<img width="276" alt="Screenshot 2024-07-16 at 10 37 02 AM"
src="https://github.com/user-attachments/assets/a5dd6e6e-5f91-4625-b225-ddfc46d5ea89">
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@AkshatJawne AkshatJawne

Labels
bug Something isn't working
Projects
None yet
Milestone
July 2024
3 participants
@mofojed @AkshatJawne @dgodinez-dh