Implement RequiresClient capability for widgets

[toger5]

Fixes: element-hq/element-web#15744
based on: matrix-org/matrix-widget-api#47 (type-check will fail without this pr)
now also requires: matrix-org/matrix-widget-api#48
The Capability RequiresClient is added. Widgets can request to not allow beeing opened in a new tab unavailable.

I am wondering if there is a better method than capabilities for situation where the widget wants to request specific preferences on how it should be treated by the matrix client. Seems to be a fairly common situation.
May "NoApprove" capabilities could be implemented as a general concept?
It indeed is a well implemented concept!! I just did not find it before...

Type: [enhancement]

---

Here's what your changelog entry will look like:

✨ Features

• Implement RequiresClient capability for widgets (#7005). Fixes Disable popout feature for widgets using the Widget API element-hq/element-web#15744 and Disable popout feature for widgets using the Widget API element-hq/element-web#15744. Contributed by @toger5.

Preview: https://617a93346092c41b431d5b1d--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

[jryans]

Thanks, looks like a good start so far! 😄

I've added some notes on the auto-approve mechanism, hopefully it's clear but let me know.

[toger5]

 this.setState({
            noPopoutButton: this.sgWidget.widgetApi.hasCapability(MatrixCapabilities.RequiresClient)
        });

Should this be noPopoutButton or requieresClient. Currently it might be a little confusing since there is also props.showPopout

[jryans]

 this.setState({
            noPopoutButton: this.sgWidget.widgetApi.hasCapability(MatrixCapabilities.RequiresClient)
        });

Should this be noPopoutButton or requieresClient. Currently it might be a little confusing since there is also props.showPopout

Is the state change needed to force re-rendering, or can you avoid state and check for the capability directly?

If you do need the state, then as with the capability name, we should avoid "no"-prefixed names as the negation makes things harder to understand. I suppose it may as well be requiresClient, since that's the name of the capability.

[jryans]

As a meta-note, I recommend requesting a fresh review when you need feedback, as the PR might be missed otherwise. To re-request a review, you can click the "refresh wheel" icon next to the reviewer's name on the top right.

[toger5]

Is the state change needed to force re-rendering, or can you avoid state and check for the capability directly?

I think the state change is needed. The AppTile is first rendered. Than the widget gets loaded, and requests the noPopout/requiresClient capability.
That is when the button disappears.
before i had the initial noPopoutButton: true, set to false. This resulted in the button beeing visible for about half a second before the rerender is triggered and it disappeared. Now the popout button only appears when the capability is not requested.

[jryans]

Thanks for working on this, we're getting closer! 😄 A few comments below.

[jryans]

Overall this is coming together. Please add the comment I've mentioned about the default state, and I expect we'll also need some final tweaks for the event name as mentioned on widget API layer.

[toger5]

About the event names.
I noticed some inconsistencies.

this.sgWidget.on("preparing", this.onWidgetPrepared);
this.sgWidget.on("ready", this.onWidgetReady);
this.sgWidget.on("capabilities_renegotiated", this.onWidgetCapabilitesRenegotiating);

"-ing" vs "-ed" (also with the callback function names)
should we rename everything to "-ed"
Wait, preparing is actually emitted when it is still preparing. renaming onWidgetPrepared to "-ing" makes more sense.

[jryans]

Wait, preparing is actually emitted when it is still preparing. renaming onWidgetPrepared to "-ing" makes more sense.

Ah yeah, seems like renaming the function makes sense in that case. For a small change like this, it seems okay to include here in this PR if you like. If it were a larger, more complex renaming, then I'd say it should happen in a separate PR instead.

[jryans]

I'll prepare a new version of matrix-widget-api and update the consumers, which hopefully should resolve the CI failures here.

[jryans]

@toger5 Could you try running yarn add matrix-widget-api@beta to upgrade the API version and commit that to your branch? Hopefully that will resolve CI here.

[toger5]

@jryans It seems to have worked. Thanks for updating the widget api!

[jryans]

Nearly there, just spotted a small typo... Once that's fixed, this should be ready to merge. 😄

[jryans]

Suggested change

	// button being visible (for an instance) and than disappearing when the widget is loaded.
	// button being visible (for an instance) and then disappearing when the widget is loaded.

[jryans]

Looks good, thanks! 😄