-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Migrate the WebSocket API page (https://xrpl.org/websocket-api-tool.html) #2265
Conversation
New and updated dependencies detected. Learn more about Socket for GitHub ↗︎
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
"longname": "amm.devnet.rippletest.net (XLS-30d AMM Devnet)" | ||
}, | ||
{ | ||
"id": "wstool-1-connection-localhost", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not for this PR but I would love for this to allow you specify a custom url like the explorer does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. Probably we can create an issue
@@ -0,0 +1,282 @@ | |||
import * as React from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd that importing React is required. The need for this was eliminated a while ago. The explorer now doesnt require it.
currentMethod, | ||
selectedConnection, | ||
}) => ( | ||
<div |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this modal code leveraging the bootstrap jquery library?
</a> | ||
</div> | ||
|
||
<div className="api-input-area pt-4"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The request area is a good candidate for its own component.
Link check report. 553607 links checked. Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/redocly-websocket-api/ |
Link check report. 553607 links checked. Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/redocly-websocket-api/ |
Link check report. 553607 links checked. Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/redocly-websocket-api/ |
"longname": "xrplcluster.com (Mainnet Full History Cluster)" | ||
}, | ||
{ | ||
"id": "connection-s2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this make sense anymore to split s1/s2 out in the list?
Link check report. 553607 links checked. Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/redocly-websocket-api/ |
Link check report. 553607 links checked. Preview: https://XRPLF.github.io/xrpl-dev-portal/pr-preview/redocly-websocket-api/ |
9998c28
to
7f51ab4
Compare
263ccc9
to
80efaf6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More translations are needed.
setKeepLast(newValue); | ||
}; | ||
|
||
const openConnectionModal = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would really like to clean this up.
const { translate } = useTranslate(); | ||
const [isConnectionModalVisible, setIsConnectionModalVisible] = | ||
useState(false); | ||
const [selectedConnection, setSelectedConnection] = useState((params.server) ? connections.find((connection) => { return connection?.ws_url === params.server }) : connections[0]); const [connected, setConnected] = useState(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I want to clean this up as well.
… out of main component.
Removed many `data-*` attributes previously used by bootstrap modal css
@mDuo13 when this is merged I recommend squash and merge. There is a lot of unimportant commits in here. If you want I can write up a new commit message or squash ahead of time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- There's a long error in the CLI and browser console about a UL, I think the right sidebar, not having unique IDs.
- The curl syntax dialog doesn't update when you modify the request or select a different example request. It seems to be stuck on the
account_channels
example forever.
Even though the FontAwesome icons are broken, I believe that is fixed in #2312 so that's fine here.
@mDuo13 @ckniffen I resolved these bullet points and most of the console error messages in this PR - #2322 |
Bug fixes for Redocly WebSocket API page
The curl dialog still seems to be broken for me, and when I open it for the first time, I see this error in the browser console:
(I'm trying this on a test branch that's using realm 0.71.1, after resolving the merge conflicts.) Weirdly, the permalink button seems to have no problems. That said, I think it's probably best to merge this as-is to get us closer to a working preview, and fix those bugs after the fact. |
Merged as 330e254 |
Migrate websocket-api-page to Redocly.
This would include:
Send Request
button to send the requestPause Subscription
button to pausesubscribe
methodsKeep Last
section to set upper bound for responses displayedThings left to do: