Skip to content

fix: DH-14657 Better disconnect handling#1261

Merged
mofojed merged 6 commits intodeephaven:mainfrom
mofojed:better-disconnect-handling
May 3, 2023
Merged

fix: DH-14657 Better disconnect handling#1261
mofojed merged 6 commits intodeephaven:mainfrom
mofojed:better-disconnect-handling

Conversation

@mofojed
Copy link
Copy Markdown
Member

@mofojed mofojed commented May 3, 2023

  • In some cases, the API disconnects and the reconnects almost immediately afterwards, causing the "Reconnecting..." modal to appear and be distracting
  • Rather than have the API lie about whether it's connected or not, UI debounces when it displays the modal, just blocking interaction with a transparent modal for the duration of the disconnection if it reconnects immediately
  • Also added logging so we can see when connected/disconnected, in case this is further observed
  • Tested using the steps in feat: JS API reconnect #1149 , and also manually setting the debounceMs to 5000 to be able to react to it

mofojed added 4 commits May 3, 2023 15:02
- This will screw up the fade transition though. Bah
- In some cases, the API disconnects and the reconnects almost immediately afterwards, causing the "Reconnecting..." modal to appear and be distracting
- Rather than have the API lie about whether it's connected or not, UI debounces when it displays the modal, just blocking interaction for the duration of the disconnection if the UI just blinks
- Also added logging so we can see when connected/disconnected, in case this is further observed
@mofojed mofojed requested a review from dsmmcken May 3, 2023 20:54
@mofojed mofojed self-assigned this May 3, 2023
@mofojed mofojed requested a review from mattrunyon May 3, 2023 21:01
@codecov
Copy link
Copy Markdown

codecov Bot commented May 3, 2023

Codecov Report

Merging #1261 (20eebfa) into main (769d753) will increase coverage by 0.00%.
The diff coverage is 76.19%.

@@           Coverage Diff           @@
##             main    #1261   +/-   ##
=======================================
  Coverage   45.57%   45.57%           
=======================================
  Files         484      486    +2     
  Lines       34077    34101   +24     
  Branches     8532     8538    +6     
=======================================
+ Hits        15531    15543   +12     
- Misses      18495    18507   +12     
  Partials       51       51           
Flag Coverage Δ
unit 45.57% <76.19%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
packages/components/src/modal/InfoModal.tsx 0.00% <0.00%> (ø)
packages/code-studio/src/main/AppMainContainer.tsx 34.58% <25.00%> (-0.49%) ⬇️
packages/components/src/modal/DebouncedModal.tsx 87.50% <87.50%> (ø)
packages/react-hooks/src/useDebouncedValue.ts 100.00% <100.00%> (ø)

... and 6 files with indirect coverage changes

@mofojed mofojed changed the title fix: Better disconnect handling fix: DH-14657 Better disconnect handling May 3, 2023
Comment thread packages/react-hooks/src/useDebouncedValue.ts Outdated
Comment thread packages/react-hooks/src/useDebouncedValue.ts Outdated
Comment thread packages/react-hooks/src/useDebouncedValue.ts
Comment thread packages/react-hooks/src/useDebouncedValue.test.ts
Comment thread packages/components/src/modal/DebouncedModal.test.tsx Outdated
mofojed and others added 2 commits May 3, 2023 17:55
Co-authored-by: Matthew Runyon <mattrunyonstuff@gmail.com>
@mofojed mofojed requested a review from mattrunyon May 3, 2023 22:19
@mofojed mofojed merged commit 9358e41 into deephaven:main May 3, 2023
@mofojed mofojed deleted the better-disconnect-handling branch May 3, 2023 23:28
@github-actions github-actions Bot locked and limited conversation to collaborators May 3, 2023
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.

2 participants