Skip to content

feat: improve table loading#1898

Merged
wusteven815 merged 9 commits intodeephaven:mainfrom
wusteven815:improve-table-loading
Apr 11, 2024
Merged

feat: improve table loading#1898
wusteven815 merged 9 commits intodeephaven:mainfrom
wusteven815:improve-table-loading

Conversation

@wusteven815
Copy link
Copy Markdown
Contributor

@wusteven815 wusteven815 commented Mar 25, 2024

  • Adds Improve loading state while seeking table #1865
    • Add a check for if there is still data being loaded in the viewport
    • Add a new loading message if the above is true for >500ms
    • Add state to determine whether startLoading will block the grid or show the cancel button

@wusteven815 wusteven815 self-assigned this Mar 25, 2024
@wusteven815
Copy link
Copy Markdown
Contributor Author

wusteven815 commented Mar 25, 2024

Didn't use a new event, wired it up to IrisGridModelUpdater's updateViewport instead

Switched to events

@wusteven815 wusteven815 linked an issue Mar 25, 2024 that may be closed by this pull request
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 52.17391% with 11 lines in your changes are missing coverage. Please review.

Project coverage is 46.11%. Comparing base (b49b506) to head (10e52f8).

Files Patch % Lines
packages/iris-grid/src/IrisGrid.tsx 50.00% 6 Missing ⚠️
...ckages/iris-grid/src/IrisGridTableModelTemplate.ts 50.00% 3 Missing ⚠️
packages/iris-grid/src/IrisGridModel.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1898   +/-   ##
=======================================
  Coverage   46.11%   46.11%           
=======================================
  Files         637      637           
  Lines       38054    38077   +23     
  Branches     9620     9626    +6     
=======================================
+ Hits        17549    17561   +12     
- Misses      20452    20463   +11     
  Partials       53       53           
Flag Coverage Δ
unit 46.11% <52.17%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wusteven815 wusteven815 marked this pull request as ready for review March 25, 2024 14:41
@wusteven815 wusteven815 requested a review from mofojed March 25, 2024 14:42
@wusteven815 wusteven815 deleted the improve-table-loading branch March 25, 2024 15:26
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 25, 2024
@wusteven815 wusteven815 restored the improve-table-loading branch March 25, 2024 15:26
@wusteven815 wusteven815 reopened this Mar 25, 2024
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iris-grid-loading element that gets loaded blocks all user interaction while scrolling... also, the Cancel button doesn't make sense in this context. It will try and rollback the state of filters etc. which doesn't make sense, and you can't really "cancel" a viewport operation.

Instead, I don't think we should show the cancel button, and we shouldn't stop the user from interacting/scrolling the table either. We just want the loading status in the bottom. So we may need to add another loading state value or something.

Comment thread packages/iris-grid/src/IrisGrid.tsx Outdated
Comment thread packages/iris-grid/src/IrisGridModel.ts
Comment thread packages/iris-grid/src/IrisGrid.tsx Outdated
Comment thread packages/iris-grid/src/IrisGrid.tsx Outdated
Comment thread packages/iris-grid/src/IrisGridTableModelTemplate.ts Outdated
@wusteven815 wusteven815 requested a review from mofojed April 3, 2024 21:15
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some cleanup/suggestions.

If we wanted to get really fancy, we'd keep a list of the pending operations and their type, then decide what loading text/spinner to display at render time based on all the current pending operations. In theory, this could also help us with our rollback before (right now we just rollback to the last loaded state). I think that's all out of scope for right now though, this is good.

Comment thread packages/iris-grid/src/IrisGridModel.ts
Comment thread packages/iris-grid/src/IrisGrid.tsx
@wusteven815 wusteven815 requested a review from mofojed April 5, 2024 16:52
Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work!

@wusteven815 wusteven815 merged commit 9b14ee0 into deephaven:main Apr 11, 2024
@wusteven815 wusteven815 deleted the improve-table-loading branch April 11, 2024 17:45
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve loading state while seeking table

2 participants