Conversation
Signed-off-by: Dennis Adjei-Baah <[email protected]>
dadjeibaah
changed the title
siggy
left a comment
siggy
left a comment
There was a problem hiding this comment.
looks good, one tioli comment...
| this.setState({ | ||
| tapRequestInProgress: false | ||
| tapRequestInProgress: false, | ||
| tapIsClosing: false |
There was a problem hiding this comment.
in most places, we're updating tapRequestInProgress and tapIsClosing together. would these two booleans make more sense / be simpler as a 3-state enum (ready, inprogress, closing)?
There was a problem hiding this comment.
That's a good point. I will add that in the next commit.
There was a problem hiding this comment.
Now that I think of it, I think it may be best to leave this as it is and create a new ticket that will address this issue and other refactor tasks that need to be done in the TopModule.
rmars
left a comment
rmars
left a comment
There was a problem hiding this comment.
nice! disabling of the Stop button seems like a very user friendly addition
| this.stopTapStreaming(); | ||
|
|
||
| if (!e.wasClean) { | ||
| if (!e.wasClean && e.code !== 1006) { |
There was a problem hiding this comment.
can we leave a comment here explaining why we're ignoring this? (maybe a link to the ticket would be helpful)
| resourcesByNs={this.state.resourcesByNs} | ||
| authoritiesByNs={this.state.authoritiesByNs} | ||
| tapRequestInProgress={this.state.tapRequestInProgress} | ||
| tapIsClosing={this.state.tapIsClosing} |
There was a problem hiding this comment.
we'll need to set a default value for this state otherwise we get a console error on the Top page:
warning.js:33 Warning: Failed prop type: The prop `tapIsClosing` is marked as required in `TapQueryForm`, but its value is `undefined`.
in TapQueryForm (created by AddUrlProps(TapQueryForm))
in AddUrlProps(TapQueryForm) (at Top.jsx:158)
in div (at Top.jsx:155)
in Top (at AppContext.jsx:10)
in Unknown (created by Route)
in Route (at index.js:56)
in Switch (at index.js:47)
in div (at index.js:46)
in div (created by Basic)
in Basic (created by Adapter)
in Adapter (at index.js:45)
in div (created by BasicLayout)
in BasicLayout (created by Adapter)
in Adapter (at index.js:43)
in div (created by BasicLayout)
in BasicLayout (created by Adapter)
in Adapter (at index.js:41)
in RouterToUrlQuery (at index.js:40)
in Router (created by BrowserRouter)
web/app/js/components/TopModule.jsx
Outdated
| startTap: PropTypes.bool.isRequired, | ||
| updateNeighbors: PropTypes.func | ||
| updateNeighbors: PropTypes.func, | ||
| updateTapClosingState: PropTypes.func.isRequired |
There was a problem hiding this comment.
I think this prop should be not required, with a default value of _.noop. The resource detail pages use the Top module, but don't use a form, so we don't need to update the form stop button in this case.
When switching from a ResourceDetail page to another page, we get a console error because that page doesn't set the updateTapClosingState function
Warning: Failed prop type: The prop `updateTapClosingState` is marked as required in `TopModule`, but its value is `undefined`.
in TopModule (at AppContext.jsx:10)
in Unknown (at ResourceDetail.jsx:218)
in div (at ResourceDetail.jsx:217)
in div (at ResourceDetail.jsx:197)
in div (at ResourceDetail.jsx:265)
in div (at ResourceDetail.jsx:264)
in ResourceDetailBase (at AppContext.jsx:10)
in Unknown (created by Route)
in Route (at index.js:53)
in Switch (at index.js:47)
in div (at index.js:46)
in div (created by Basic)
in Basic (created by Adapter)
Signed-off-by: Dennis Adjei-Baah <[email protected]>
… other page Signed-off-by: Dennis Adjei-Baah <[email protected]>
3 participants
When a websocket connection is closed between Chrome and a server, we get a
1006error code signifying abnormal closure of the websocket connection. It seems as if we only get this error on Chrome web clients. Firefox and Safari do not encounter this issue.The solution is to suppress
1006errors that occur in the web browser since the connection is closed anyway. There is no negative side effect that occurs when the connection is closed abnormally and so the error message is benign.fixes #1630
Signed-off-by: Dennis Adjei-Baah [email protected]