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

Feat/record mode #5718

Merged
merged 9 commits into from
Nov 23, 2022
Merged

Conversation

djballowe
Copy link
Contributor

This PR looks to solve a pain for folks trying to do screen captures in Hubs.

Currently, we have a hide ui function but it fails to hide the cursor, debug menus and is not accessible on non-QWRTY keyboards.
Additionally it's annoying to have to close the hide ui tip manually each time you want to record.
This solution addresses all of the previously mentioned problems and provides a smooth transition to a state suitable for recording.

Alternate solutions we considered:

we thought about hiding nametags as well but decided to leave that up to the user.
we considered different inputs on the off chance a user presses the key by mistake. We decided 'R' was too close to movement keys.
we thought about using the built in toast messages but this got tricky since we were also using the built in functions for hide UI

record.mp4

djballowe and others added 8 commits August 6, 2022 20:32
>
>
Co-authored-by: Matt Cool <matthewbcool@gmail.com>
Co-authored-by:matthewbcool@gmail.com
Co-authored-by:ben_rooke@icloud.com
As I mentioned at the collaborative hacking session on Saturday, here is some code to make the cursor invisible without changing the scale.
While there isn't much difference in the end result, I do want to note that with the scaling method, the cursor will show up again when it hovers over an object, but making it invisible will keep it hidden (the blue flashing glow is present with both methods).
Make cursor invisible instead of scaling
add toast notification for record mode
Copy link
Contributor

@netpro2k netpro2k left a comment

Choose a reason for hiding this comment

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

This LGTM. Thanks for the PR!

The "b" keybinding seems fine for now but we may end up moving it elsewhere at some point.

@netpro2k
Copy link
Contributor

netpro2k commented Nov 2, 2022

Actually it looks like this now has some merge conflicts. I suspect its from the changes made with chat/mic/moderation.

@djballowe
Copy link
Contributor Author

@netpro2k it should be resolved now and ready to merge if everything looks good to you!

@matthewbcool
Copy link
Contributor

Tested locally without issues. lgtm

Copy link
Contributor

@netpro2k netpro2k left a comment

Choose a reason for hiding this comment

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

LGTM

@matthewbcool matthewbcool merged commit cba6296 into Hubs-Foundation:master Nov 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants