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

[FAB-135] Participant control #60

Merged
merged 19 commits into from
Oct 9, 2021
Merged

[FAB-135] Participant control #60

merged 19 commits into from
Oct 9, 2021

Conversation

daystram
Copy link
Contributor

@daystram daystram commented Oct 7, 2021

  • Kick participants
  • Clear controller layer
  • LayerToolbar readability

@daystram daystram temporarily deployed to Development October 7, 2021 11:18 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 11:18 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 11:22 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 11:54 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 11:54 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 11:57 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:20 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:20 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:23 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:32 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:32 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 12:35 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 13:11 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 13:11 Inactive
@daystram daystram temporarily deployed to Development October 7, 2021 13:14 Inactive
Copy link
Member

@nicolauscg nicolauscg left a comment

Choose a reason for hiding this comment

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

changes looks good, I tested on localhost for the new feats this PR and ur tool improvements PR, reviewed changes line by line for this one too.

to confirm ws flow

  • for clear: hub sends ws msg which BE will broadcast to student's canvas and hub layer for that role
  • for kick: hub sends ws msg then BE will dc that student? if so, we can put some text on ws flow confluence page so others may be aware of this

2 minor stuff for discussion

  • do we want color tool tooltip to be 1 or 2 rows? concerned of a possbile overflow if 1 row
  • looks like tooltip border for both hub and student tools is hard to see when canvas is also white, do we want to try adding like a black border?

@daystram
Copy link
Contributor Author

daystram commented Oct 9, 2021

Both new WS flow appears correct, will update docs. We can have 2 rows for the colour palette and add a thin border to the tooltip.

@sonarcloud
Copy link

sonarcloud bot commented Oct 9, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Copy link
Member

@nicolauscg nicolauscg left a comment

Choose a reason for hiding this comment

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

all good!

@daystram daystram merged commit 9ca5182 into master Oct 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants