Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Bi-directional widget postMessaging API (stickerpacks) [WIP] #1672

Merged
merged 177 commits into from
Apr 3, 2018

Conversation

rxl881
Copy link
Contributor

@rxl881 rxl881 commented Dec 18, 2017

Extend the widget postMessaging API to allow bi-directional communication with widgets.

In part this adds the ability for widgets to pass their capabilities to matrix clients, so that client-side UI elements can be rendered appropriately.

@ara4n, @turt2live - This should be ready for (further) review now.

package.json Outdated
@@ -58,6 +58,7 @@
"classnames": "^2.1.2",
"commonmark": "^0.27.0",
"counterpart": "^0.18.0",
"dom-to-image": "^2.6.0",
Copy link
Member

Choose a reason for hiding this comment

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

This isn't actually used at this level btw

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Yep, cruft that needs to be removed.

- No additional fields.
Response:
{
screenshot: {data...}
Copy link
Member

Choose a reason for hiding this comment

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

What is data? a blob?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, a blob. I'll update the comment to reflect this.

@rxl881 rxl881 changed the title Bi-directional widget postMessaging API (screenshots) Bi-directional widget postMessaging API (screenshots) [WIP] Dec 19, 2017
@rxl881 rxl881 changed the title Bi-directional widget postMessaging API (screenshots) [WIP] Bi-directional widget postMessaging API (screenshots) Dec 28, 2017
@rxl881 rxl881 requested a review from ara4n December 28, 2017 22:39
@ara4n
Copy link
Member

ara4n commented Jan 3, 2018

this looks fairly sane to me from a high level review - although it also looks as if this is separate from the scalar-facing postmessage API for integrations, rather than widgets? I assumed that by creating a base class for postmessage APIs it'd be extended both for widgets & integs?

@rxl881
Copy link
Contributor Author

rxl881 commented Apr 2, 2018

@dbkr -- I think that I have now addressed everything from your review. Can you please have another quick check. Hopefully this is now ready to go?

@rxl881 rxl881 assigned dbkr and unassigned rxl881 Apr 2, 2018
@rxl881
Copy link
Contributor Author

rxl881 commented Apr 2, 2018

@dbkr -- One other thing (as discussed last week, I think), I'm assuming that the build errors from travis-ci can be ignored in this instance, right?

@ara4n
Copy link
Member

ara4n commented Apr 3, 2018

i'm not seeing any build errors any more, so i think you're okay on that score... i assume it was the CI building against the wrong branches of the various projs.

@ara4n
Copy link
Member

ara4n commented Apr 3, 2018

@rxl881 it looks like you've addressed all of @dbkr's comments (dave: huge thanks for doing a comprehensive review), this lgtm :)

@rxl881 rxl881 merged commit 06f1c50 into develop Apr 3, 2018
@rxl881 rxl881 deleted the rxl881/snapshot branch April 3, 2018 20:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants