Hubs-Foundation / hubs

Duck-themed multi-user virtual spaces in WebVR. Built with A-Frame.
https://hubsfoundation.org
Mozilla Public License 2.0
2.14k stars 1.41k forks source link

HUD Invite button: fixes by calling Web Share API directly #6510

Closed DougReeder closed 3 weeks ago

DougReeder commented 1 month ago

The in-world Invite button is currently broken, because it emits the "action_invite" event, but the handler is removed along with the rest of the 2-D UI, when entering immersive mode.

As the user can always copy the Invite URL/Room URL when in 2-D mode, I'm assuming the in-world Invite button should be a quick shortcut. Therefore, I have wired it up to call the Web Share API directly. This reduces the friction of inviting someone to join you when in immersive mode in a headset, thus advancing our goal of making it easy for multiple people to join one instance.

Tested on a Quest 3.

This PR is dependent on #6508

DougReeder commented 1 month ago

That completes the changes from today's dev meeting.

Exairnous commented 3 weeks ago

@DougReeder This generally looks good to me (I don't have a VR headset to test, but everything seems okay), but I'm wondering a bit about the revisions to the try/catch statements. Is there a reason why the try/catch couldn't have been left in the shareInviteUrl function? The VR path seems to be the only one with a try/catch outside the shareInviteUrl function, so that seems to suggest that both the function and the enclosing VR setup part have the risk of failing, so it seems like it would be useful to know where it failed, rather than lumping it all in together. This also adds more code complexity, so it would be nice to avoid that if possible, but if there are reasons for the additional complexity then it's fine to leave it like this.

DougReeder commented 3 weeks ago

Yes, I wanted to keep the try-catch in shareInviteUrl(), but when sharing throws an error (most commonly, because the user has cancelled), await handleExitTo2DInterstitial(true, () => {}); must not be called.

If there's a way to clean up after exiting immersive mode and sharing, that works whether sharing throws an exception or not, that would be great!

Exairnous commented 3 weeks ago

Hmm, that does complicate things. What about returning a boolean value from shareInviteUrl() depending on whether an exception was thrown and then testing for a true return value before calling await handleExitTo2DInterstitial(true, () => {});? E.g.

export async function shareInviteUrl(intl, url, values = {}, inEnglish = false, event) {
  try {
    // snip
    return true;
  } catch (error) {
    console.error("unable to share:", error);
    return false;
  }
}

let success = await shareInviteUrl();
if (success) {
  await handleExitTo2DInterstitial(true, () => {});
}
Exairnous commented 3 weeks ago

Thanks for the changes (and the PR). LGTM. Merging.