-
Notifications
You must be signed in to change notification settings - Fork 0
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
Use STOMP on top of websockets to receive events in real time #8
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This makes its purpose a lot more clear.
No more waiting 10 seconds before anything shows up
Instead of calling towers.get(towerId) a bunch of time, only bother calling it once and hanging onto that instance of ObservableTower.
We never have a list of messages to provide to ObservableChannel at construction time, nor do we ever have a list of channels to pass to an ObservableTower at construction time. As such, they shouldn't exist as construction parameters.
Apparently, the place where you call makeAutoObservable in the constructor is really important. I was calling it at the beginning but since the fields were technically uninitialized at that point, the values were never being observed properly and re-renders weren't happening even when the state changed (such as when a message was edited).
Channels will now display in order of their order property. The ordering will be from the lowest number to highest, from top to bottom such that channels at the top have the lowest order number and those at the bottom have the highest.
I plan to use it later >:)
Sciman101
approved these changes
Apr 5, 2022
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.
Looks good
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In trying to figure out what was the best way to use react-stomp in the frontend such that we could have a straightforward way to subscribe to all the topics relevant to each of the towers the user is a member of. What I settled on was sneakily using the
<SockJsClient>
component in each element of theTowerList
component. I extracted what was a simple<li>
entry into its own component calledTowerListElement
. This still returns a simple<li>
as part of a React fragment (<> </>
) that also contains<SockJsClient>
(which doesn't render any visible content so it doesn't mess anything up).Just like #7, this includes a lot of changes but doesn't fundamentally change anything about the structure of the app. The only really big change is
TowerListElement
. Like IncPlusPlus/beacon-backend#34, I suggest you look at the changes individually per commit instead of all at once since this is kind of big.Also, feel free to view the deployment created for this PR. Just know that it won't work properly unless EITHER
cisBasePath
local storage value tohttps://beacon-cis-pr-34.herokuapp.com/
ORSomething you'll notice in the console from now on is the following warning. This is a known issue with react-stomp. See lahsivjar/react-stomp#173.
If you're curious about the messages being passed around and how to see them, open up the
Network
tab of your browser's dev tools and refresh the page. Look for the one with the name "websocket" and click on it. If you don't see the websocket request, you may have opened the dev tools after the connection was already established, in which case you can just reload the page and the connection will be re-established before your very eyes.Next, click the
Messages
tab.You can now click on any message to see its content.
Once again, please ask questions about literally anything.