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

site: set zoom buttons in DepthChart ctor #1799

Merged
merged 1 commit into from
Aug 19, 2022
Merged

Conversation

chappjc
Copy link
Member

@chappjc chappjc commented Aug 18, 2022

This resolves the error Uncaught TypeError: can't access property "setExtents", e.zoomOutBttn is undefined, which happens often on DEX server reconnect events. Probably more so if you have more than one server configured.
The best way to reproduce is:

  • get the dex window opened to the markets page
  • switch to another tab in the same window, effectively hiding the tab
  • minimize the window for good measure, but neither of these steps are really required
  • wait for a reconnect, or wait a minute or two and force a reconnect (e.g. restart tor, toggle vpn, etc).

The root of the issue seems to be that the parent constructor was not setting the zoom buttons as intended. And the requestAnimation callback that actually was setting them via the resized reporter is not called until the browser decides to actually draw on the canvas. This is asynchronous, and the steps above try to make it a longer delay.

The zoom buttons were not being initialized in the constructor as
indended. This calls setZoomBttns directly from the constructor of the
inheriting class, DepthChart.

This also fixes a missing ws.deregisterRoute for updateRemainingRoute.

The zoom buttons were not being initialized in the constructor as
indended. This calls setZoomBttns directly from the constructor of the
inheriting class, DepthChart.

This also fixes a missing ws.deregisterRoute for updateRemainingRoute.
Copy link
Member

@buck54321 buck54321 left a comment

Choose a reason for hiding this comment

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

I spent a good deal of time trying to repro this and was unable to, but this change is righteous. Better to not assign child-only properties in the parent class constructor.

@chappjc
Copy link
Member Author

chappjc commented Aug 19, 2022

Looks like with webpack --watch, the parent constructor does seem to affect the child class, but not when fully compiled into a webpack bundle. The dev.js config with watch doesn't use babel, so maybe transpiling changes something.

My guess is that it has something to do with how the TypeScript class field declarations are made into js. https://jsfiddle.net/bf9w410v/

Copy link
Member

@JoeGruffins JoeGruffins left a comment

Choose a reason for hiding this comment

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

I was able to reproduce the error with a method suggested by chapp on simnet turning off the server while not looking at the tab and letting it reconnect:

image

However, I cannot seem to make it happen with these changes.

@chappjc chappjc merged commit 07a74fa into decred:master Aug 19, 2022
@chappjc chappjc deleted the setzoombtns branch August 19, 2022 15:28
@chappjc chappjc added this to the 0.5 milestone Aug 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants