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

Locked down demo mvp #6030

Merged
merged 13 commits into from
Apr 24, 2023
Merged

Locked down demo mvp #6030

merged 13 commits into from
Apr 24, 2023

Conversation

mikemorran
Copy link
Contributor

Implementing time-boxed version of simplified demo UI to be released internally. This feature does the following for any hub_id specified as demo room in the admin panel...

  • Bypass profile setup step of entry flow (avatar and display name selection)
  • Force individual profile settings for default avatar, random name generation, and always hide nametag
  • Disable UI elements for...
  1. Presence Log when new users enter and leave
  2. Chat button
  3. Share button
  4. Place button
  5. Microphone button
  6. Object Menu
  7. People Menu hover and clickability
  8. "Change Name and Avatar" in the more menu
  • Disable "T" and "/" hotkeys which open chat
  • Prevent separate tab avatar changes with avatar.js

Public release of this feature will occur once room info and settings for a demo room can be programmatically specified in the admin panel. Also, additional edit should be implemented to support an array of demo room hub_ids.

Specific Feedback Requested:
@keianhzo I was unable to solve checking for canVoiceChat before our hack to request mic when using Safari. This is because the hack requests permissions before we ask reticulum for the hub_id or preferences. Any input on why we request mic permissions so early?

@keianhzo I implemented a pretty sloppy hack in store.js to prevent users from updating their avatarId from separate tabs using the avatar.js panel. Any thoughts about how this could be better accomplished, especially since it is using AFRAME?

@nikk15 @nickgrato Any thoughts on how I removed pointer-events on the People Menu in ui-root.js, src/react-components/room/ContentMenu.js, and src/react-components/room/ContentMenu.scss? Could this be improved?

nikk15
nikk15 previously requested changes Apr 12, 2023
Copy link
Contributor

@nikk15 nikk15 left a comment

Choose a reason for hiding this comment

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

I think installing the Prettier extension in VSCode should solve some of the linting issues too. I have my VSCode settings set to "format on save" :)

src/react-components/ui-root.js Outdated Show resolved Hide resolved
src/react-components/ui-root.js Show resolved Hide resolved
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

This is close to be ready. I have left a few comments, let me know if you need more context.

@keianhzo I was unable to solve checking for canVoiceChat before our hack to request mic when using Safari. This is because the hack requests permissions before we ask reticulum for the hub_id or preferences. Any input on why we request mic permissions so early?

Yeah, it's unfortunate that we still need to do this. It might have been already fixed and we can maybe remove it. I'll test. For this PR I think it's ok to leave it as it is, we can remove it in a follow-up in case it's not necessary anymore.

@keianhzo I implemented a pretty sloppy hack in store.js to prevent users from updating their avatarId from separate tabs using the avatar.js panel. Any thoughts about how this could be better accomplished, especially since it is using AFRAME?

Do we want to let the user choose an avatar during entry? If we don't care about that we can maybe switch this to:

    if (isLockedDownDemoRoom()) {
      return;
    }

Otherwise you can avoid using the AFRAME symbol by using:

if (
      newState.profile !== undefined &&
      newState.profile.avatarId !== undefined &&
      isLockedDownDemoRoom() &&
      APP.scene.is("entered")
    ) {
      return;
    }

We need to migrate that check at some point so it's ok to use that.

src/react-components/room/ChatSidebar.js Outdated Show resolved Hide resolved
src/react-components/ui-root.js Outdated Show resolved Hide resolved
src/components/hud-controller.js Show resolved Hide resolved
src/react-components/preferences-screen.js Show resolved Hide resolved
src/react-components/room/ContentMenu.scss Outdated Show resolved Hide resolved
src/systems/ui-hotkeys.js Outdated Show resolved Hide resolved
@mikemorran
Copy link
Contributor Author

@keianhzo Re: hack to prevent avatar selection in a separate tab.

I am attempting to allow default avatar selection on initial connection, not avatar selection in the entry flow, before disabling any avatar changes. This is important to force all users joining a demo room to choose from a default avatar and not bring in an avatar from their previous session in a different room. Since avatar.js does not specify that avatarID was changed in a separate tab, I am doing this weird hack to only allow avatarID changes on initial connection to a demo room (hence trying to judge when the scene is loaded). Happy to discuss further!

Copy link
Contributor Author

@mikemorran mikemorran left a comment

Choose a reason for hiding this comment

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

Feedback implemented!

@mikemorran
Copy link
Contributor Author

I think installing the Prettier extension in VSCode should solve some of the linting issues too. I have my VSCode settings set to "format on save" :)

Implemented

@mikemorran mikemorran closed this Apr 17, 2023
@mikemorran
Copy link
Contributor Author

Re-opening accidental close

@mikemorran mikemorran reopened this Apr 17, 2023
@mikemorran mikemorran dismissed stale reviews from keianhzo and nikk15 April 17, 2023 19:23

Changed implemented!

Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

I've made a couple more suggestion to try to minimize the changes impact. I think after this we should be good to go!

src/react-components/ui-root.js Outdated Show resolved Hide resolved
src/storage/store.js Outdated Show resolved Hide resolved
src/hub.js Outdated Show resolved Hide resolved
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

I think when we remove those two asycs that were left we are good.

src/hub.js Outdated Show resolved Hide resolved
src/hub.js Outdated Show resolved Hide resolved
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

Nice! this looks ready to ship after a QA pass.

@mikemorran mikemorran merged commit df35431 into master Apr 24, 2023
19 of 23 checks passed
@mikemorran mikemorran deleted the LockedDownDemo branch April 24, 2023 18:26
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.

None yet

5 participants