-
-
Notifications
You must be signed in to change notification settings - Fork 8
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
Configure widget permissions using ModuleRunner.instance.invoke #11
Configure widget permissions using ModuleRunner.instance.invoke #11
Conversation
Signed-off-by: Mikhail Aheichyk <[email protected]>
@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! |
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. |
Signed-off-by: Mikhail Aheichyk <[email protected]>
@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. |
There was a problem hiding this 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.
Signed-off-by: Mikhail Aheichyk <[email protected]>
@andybalaam thanks for the feedback! Test was added, please have a look. |
If test is fine, would be great maybe to have a new version of |
There was a problem hiding this 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!
@maheichyk new release 0.0.4 published at https://www.npmjs.com/package/@matrix-org/react-sdk-module-api |
This PR suggests changes to module API to support customization of widget permissions using
ModuleRunner.instance.invoke
as discussed here: #9 (comment)