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.

Sign up for GitHub

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

Jump to bottom

Configure widget permissions using ModuleRunner.instance.invoke #11

Merged
merged 3 commits into from
Feb 16, 2023
Merged

Configure widget permissions using ModuleRunner.instance.invoke #11

merged 3 commits into from
Feb 16, 2023

Conversation

maheichyk
Copy link
Contributor

This PR suggests changes to module API to support customization of widget permissions using ModuleRunner.instance.invoke as discussed here: #9 (comment)

widget permissions using "events" object
59c1a71 
Signed-off-by: Mikhail Aheichyk <mikhail.aheichyk@nordeck.net>
@maheichyk maheichyk requested a review from a team as a code owner February 9, 2023 09:43
@maheichyk maheichyk mentioned this pull request Feb 9, 2023
Merged
@richvdh
Copy link
Member

richvdh commented Feb 9, 2023

@andybalaam it looks like you've already done a bunch of thinking about this, so I'm afraid I'm going to throw it over the wall in your direction!

@richvdh richvdh requested review from andybalaam and removed request for richvdh February 9, 2023 10:08
@maheichyk maheichyk mentioned this pull request Feb 9, 2023
Closed
@maheichyk
Copy link
Contributor Author

Hi @andybalaam could you please provide feedback on this PR?

@andybalaam
Copy link
Contributor

Hi @andybalaam could you please provide feedback on this PR?

@maheichyk I'm sorry for the delay on this. I will take a look today.

lint fix
e5187a9 
Signed-off-by: Mikhail Aheichyk <mikhail.aheichyk@nordeck.net>
@andybalaam
Copy link
Contributor

@maheichyk this looks great. Please could we have a small test similar to what you did in #9 and then we'll be ready to merge this.

Thanks again for your work on this and your patience.

andybalaam
andybalaam requested changes Feb 15, 2023
View reviewed changes
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

Looks good - requesting a test that validates the flow.

WidgetLifecycle test
4a4bff2 
Signed-off-by: Mikhail Aheichyk <mikhail.aheichyk@nordeck.net>
@maheichyk
Copy link
Contributor Author

@andybalaam thanks for the feedback! Test was added, please have a look.

@maheichyk
Copy link
Contributor Author

If test is fine, would be great maybe to have a new version of matrix-react-sdk-module-api that can be used to build this PR matrix-org/matrix-react-sdk#10121

andybalaam
andybalaam approved these changes Feb 16, 2023
View reviewed changes
Copy link
Contributor

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

This looks great, thank you!

@andybalaam andybalaam merged commit d9b2ec0 into matrix-org:main Feb 16, 2023
@andybalaam
Copy link
Contributor

@maheichyk new release 0.0.4 published at https://www.npmjs.com/package/@matrix-org/react-sdk-module-api

@dhenneke dhenneke deleted the module_widget_permissions_event branch August 18, 2023 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andybalaam andybalaam andybalaam approved these changes

@weeman1337 weeman1337 Awaiting requested review from weeman1337 weeman1337 is a code owner automatically assigned from matrix-org/element-web-reviewers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@maheichyk @richvdh @andybalaam @benparsons