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.

Sign up for GitHub

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

Jump to bottom

Add UI websocket open/close and send/receive #728

Merged
merged 8 commits into from
Apr 26, 2019
Merged

Add UI websocket open/close and send/receive #728

merged 8 commits into from
Apr 26, 2019

Conversation

darkodraskovic
Copy link
Contributor

Signed-off-by: Darko Draskovic darko.draskovic@gmail.com

@codecov-io
Copy link

codecov-io commented Apr 22, 2019 •
edited
Loading

Codecov Report

Merging #728 into master will decrease coverage by 0.04%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #728      +/-   ##
==========================================
- Coverage   85.13%   85.08%   -0.05%     
==========================================
  Files          64       64              
  Lines        4124     4124              
==========================================
- Hits         3511     3509       -2     
- Misses        415      417       +2     
  Partials      198      198
Impacted Files Coverage Δ
things/service.go 90.79% <0%> (-1.23%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc93332...966731d. Read the comment docs.

drasko
drasko requested changes Apr 22, 2019
View reviewed changes
ui/src/Message.elm Outdated
@@ -146,13 +188,15 @@ view model =
[ Input.text [ Input.id "message", Input.onInput SubmitMessage ]
]
, Button.button [ Button.secondary, Button.attrs [ Spacing.ml1 ], Button.onClick SendMessage ] [ text "Send" ]
, Button.button [ Button.secondary, Button.attrs [ Spacing.ml1 ], Button.onClick WebsocketSend ] [ text "Websocket" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the tet is "Websocket"? It would be better that it is something like "Recieve" or "Listen" - user does not have to know really about underlaying technology.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, you need to transform this button in something like "Stop" or something, to stop listening (i.e. unsubscribe, i.e. close this websocket),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in the last commit.

ui/src/Websocket.js Outdated
if (wss[url]) {
wss[url].close();
} else {
MF.log("Websocket is not open. URL: " + url);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this comment OK? Looks like a copy of precious one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it just informs the user that the websocket he's trying to close is not open, so he can't close it. I can reword the text so that it becomes more clear.

@darkodraskovic darkodraskovic force-pushed the websocket branch 3 times, most recently from f5d76be to 7131dce Compare April 24, 2019 11:56
blokovi
blokovi previously approved these changes Apr 24, 2019
View reviewed changes
Copy link
Contributor

@blokovi blokovi left a comment

Choose a reason for hiding this comment

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

LGTM

manuio
manuio suggested changes Apr 24, 2019
View reviewed changes
ui/src/Websocket.js Outdated
app.ports.connectWebsocket.subscribe(function(data) {
var url = MF.url(data);
if (wss[url]) {
MF.log("Websocket already open. URL: " + url );
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you use '' instead of "" here and in the rest of the code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The idea was to use single quotes for the things that happen under the hood and to use the double quotes for the messages intended for the user.

manuio
manuio previously approved these changes Apr 25, 2019
View reviewed changes
drasko
drasko previously approved these changes Apr 25, 2019
View reviewed changes
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

manuio
manuio previously approved these changes Apr 25, 2019
View reviewed changes
@darkodraskovic
Add websocket open/close and send/receive
ddac69c 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Add websocket feedback/log to Elm
a091b86 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Align WS open/close buttons
761cd5d 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Add websocket query for channel list
5b62ccd 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Add websocket related encoders/decoders for port interops
b0f41da 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Fix .js indentation and simplify ws (dis)connect method
983a5b2 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Use single quotes consistently
66f3dad 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic
Add HTTP Send and WS Listen
966731d 
Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
@darkodraskovic darkodraskovic dismissed stale reviews from manuio and drasko via 966731d April 26, 2019 16:37
drasko
drasko approved these changes Apr 26, 2019
View reviewed changes
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@manuio manuio merged commit 700cfdf into absmach:master Apr 26, 2019
@darkodraskovic darkodraskovic deleted the websocket branch April 29, 2019 09:27
davide83 pushed a commit to davide83/mainflux that referenced this pull request May 13, 2019
@darkodraskovic @davide83
Add UI websocket open/close and send/receive (absmach#728)
35a2dfd 
* Add websocket open/close and send/receive

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket feedback/log to Elm

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Align WS open/close buttons

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket query for channel list

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket related encoders/decoders for port interops

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Fix .js indentation and simplify ws (dis)connect method

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Use single quotes consistently

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add HTTP Send and WS Listen

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
rugwirobaker pushed a commit to rugwirobaker/mainflux that referenced this pull request Jun 26, 2019
@darkodraskovic @rugwirobaker
Add UI websocket open/close and send/receive (absmach#728)
3a7cdd3 
* Add websocket open/close and send/receive

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket feedback/log to Elm

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Align WS open/close buttons

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket query for channel list

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket related encoders/decoders for port interops

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Fix .js indentation and simplify ws (dis)connect method

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Use single quotes consistently

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add HTTP Send and WS Listen

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
manuio pushed a commit that referenced this pull request Oct 12, 2020
@darkodraskovic @manuio
Add UI websocket open/close and send/receive (#728)
c37cbf9 
* Add websocket open/close and send/receive

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket feedback/log to Elm

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Align WS open/close buttons

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket query for channel list

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add websocket related encoders/decoders for port interops

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Fix .js indentation and simplify ws (dis)connect method

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Use single quotes consistently

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>

* Add HTTP Send and WS Listen

Signed-off-by: Darko Draskovic <darko.draskovic@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drasko drasko drasko approved these changes

@manuio manuio manuio approved these changes

@blokovi blokovi blokovi left review comments

Assignees
No one assigned
Labels
feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@darkodraskovic @codecov-io @drasko @manuio @blokovi