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

Add RequiresClient capability #47

Merged
merged 2 commits into from
Oct 26, 2021

Conversation

toger5
Copy link
Contributor

@toger5 toger5 commented Oct 21, 2021

This PR only adds RequiresClient to the Capability enum.
Required for: matrix-org/matrix-react-sdk#7005

@toger5 toger5 requested a review from a team as a code owner October 21, 2021 16:47
@jryans
Copy link
Contributor

jryans commented Oct 21, 2021

Unsure if we have it recorded as a style rule, but generally we tend to name PRs in the style "[Title case, active verb] blah blah[no full stop]", so perhaps "Add NoPopout capability" here.

@toger5 toger5 changed the title add NoPopout capability. Add NoPopout capability Oct 21, 2021
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

Otherwise, lgtm! Thanks!

src/interfaces/Capabilities.ts Outdated Show resolved Hide resolved
@toger5 toger5 force-pushed the no_popout_capability_15744 branch from 662c500 to 291bb58 Compare October 22, 2021 11:30
@toger5 toger5 changed the title Add NoPopout capability add RequiresClient capability Oct 22, 2021
@toger5 toger5 requested a review from jryans October 22, 2021 11:32
@toger5 toger5 changed the title add RequiresClient capability Add RequiresClient capability Oct 25, 2021
@@ -20,7 +20,8 @@ export enum MatrixCapabilities {
Screenshots = "m.capability.screenshot",
StickerSending = "m.sticker",
AlwaysOnScreen = "m.always_on_screen",

// Ask Element to not give the option to move the widget into a seperate tab.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// Ask Element to not give the option to move the widget into a seperate tab.
// Ask Element to not give the option to move the widget into a separate tab.

@jryans
Copy link
Contributor

jryans commented Oct 25, 2021

Once the typo is fixed, I think this side is ready to go, so request another review once that's done.

@toger5 toger5 requested a review from jryans October 25, 2021 15:46
Copy link
Contributor

@jryans jryans left a comment

Choose a reason for hiding this comment

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

This part looks good to me, thanks for working on it! 😄

I'll merge this side now, as that may help with type failures in CI on the React SDK side.

@jryans jryans merged commit 44bdf6e into matrix-org:master Oct 26, 2021
@jryans
Copy link
Contributor

jryans commented Oct 26, 2021

@turt2live Will we need a release of this layer and update to the new version before the React SDK side is happy?

@toger5 toger5 deleted the no_popout_capability_15744 branch October 26, 2021 11:48
@turt2live
Copy link
Member

Almost certainly, yea. Both react-sdk and element-web will need package updates.

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.

3 participants