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 event type for stickers [WIP] #590

Merged
merged 3 commits into from
Jan 9, 2018
Merged

Add event type for stickers [WIP] #590

merged 3 commits into from
Jan 9, 2018

Conversation

rxl881
Copy link
Contributor

@rxl881 rxl881 commented Jan 4, 2018

No description provided.

@rxl881 rxl881 changed the base branch from master to develop January 4, 2018 22:45
@rxl881 rxl881 self-assigned this Jan 4, 2018
@rxl881 rxl881 requested a review from ara4n January 8, 2018 11:20
@rxl881 rxl881 assigned ara4n and unassigned rxl881 Jan 8, 2018
@ara4n
Copy link
Member

ara4n commented Jan 8, 2018

lgtm - sorry for the spectacular fuckup there :( serves me right for blindly porting from python and not writing any UTs for it - sorry :/

@rxl881
Copy link
Contributor Author

rxl881 commented Jan 9, 2018

No worries at all. Glad to have got to the bottom of it ... I was staring at that code for a little while trying to understand how exactly it was intended to work :)

@rxl881 rxl881 merged commit f404356 into develop Jan 9, 2018
@rxl881 rxl881 deleted the rxl881/stickers branch January 9, 2018 14:49
@Half-Shot
Copy link
Contributor

Half-Shot commented Jan 18, 2018

Thanks for taking the time with getting stickers going, it's been a long time coming. With that being said, I've got several issues with this.

I can't see how this differs from an image event, other than the metadata of it being a sticker which says...that I am meant to render it slightly smaller. There is also zero information on what info is meant to be. I'm assuming it's a ImageInfo type, which brings me back to point one.

Anyway, besides that there is no spec for this. Nothing merged, nothing pending. No explanation of what a sticker even is in this context. If it's my assumption, a telegram sticker then it is missing a LOT of information like which pack it is part of and which emoji it might correspond to.

It could also mean another type of sticker altogether that I am not aware of, in which case it is vital that there is a spec explaining.
This may be WIP (and I'd even go as far as to argue a WIP implementation should not be merged as a feature), but it certainly should already be clearly distinct from existing types from the get go.

I want to reiterate that stickers are something we very much want to have, but it also has to be carefully controlled and crafted or we will end up having to rewrite this event type several times and it really aught to have a gdoc to go with it.

@ara4n
Copy link
Member

ara4n commented Jan 18, 2018

i think all these concerns can be allayed with a spec proposal googledoc, which we'll rustle up asap. the m.room.sticker event is very much experimental atm (and probably should be im.vector.sticker or something whilst we mess with it, although that'd probably cause hysteria that we were trying to make stickers riot-specific or something daft).

To quickly answer the questions: using a differrent event type is a deliberate move towards FINALLY solving the human representation of arbitrary events "problem".

The difference with m.room.message-with-msgtype-image is that clients are meant to display it inline without any lightboxing or download links or metadata info like you might do for an image. It also spells out the necessary semantics.

In terms of whether we link back to the source stickerpack or give it an emoji equivalent - that sounds like very useful stuff; suggestions welcome :) Let's go graffiti on a spec proposal as soon as there's one written up out of the convos which produced the current experiment.

@rxl881 rxl881 restored the rxl881/stickers branch February 7, 2018 09:34
krombel added a commit to krombel/matrix-js-sdk that referenced this pull request Mar 21, 2018
[Full Changelog](matrix-org/matrix-js-sdk@v0.9.2...v0.10.0-rc.1)
* Fix duplicated state events in timeline from peek
[\matrix-org#630](matrix-org#630)
* Create indexeddb worker when starting the store
[\matrix-org#627](matrix-org#627)
* Fix indexeddb logging
[\matrix-org#626](matrix-org#626)
* Don't do /keys/changes on incremental sync
[\matrix-org#625](matrix-org#625)
* Don't mark devicelist dirty unnecessarily
[\matrix-org#623](matrix-org#623)
* Cache the joined member count for a room state
[\matrix-org#619](matrix-org#619)
* Fix JS doc
[\matrix-org#618](matrix-org#618)
* Precompute push actions for state events
[\matrix-org#617](matrix-org#617)
* Fix bug where global "Never send to unverified..." is ignored
[\matrix-org#616](matrix-org#616)
* Intern legacy top-level 'membership' field
[\matrix-org#615](matrix-org#615)
* Don't synthesize RR for m.room.redaction as causes the RR to go missing.
[\matrix-org#598](matrix-org#598)
* Make Events create Dates on demand
[\matrix-org#613](matrix-org#613)
* Stop cloning events when adding to state
[\matrix-org#612](matrix-org#612)
* De-dup code: use the initialiseState function
[\matrix-org#611](matrix-org#611)
* Create sentinel members on-demand
[\matrix-org#610](matrix-org#610)
* Some more doc on how sentinels work
[\matrix-org#609](matrix-org#609)
* Migrate room encryption store to crypto store
[\matrix-org#597](matrix-org#597)
* add parameter to getIdentityServerUrl to strip the protocol for invites
[\matrix-org#600](matrix-org#600)
* Move Device Tracking Data to Crypto Store
[\matrix-org#594](matrix-org#594)
* Optimise pushprocessor
[\matrix-org#591](matrix-org#591)
* Set event error before emitting
[\matrix-org#592](matrix-org#592)
* Add event type for stickers [WIP]
[\matrix-org#590](matrix-org#590)
* Migrate inbound sessions to cryptostore
[\matrix-org#587](matrix-org#587)
* Disambiguate names if they contain an mxid
[\matrix-org#588](matrix-org#588)
* Check for sessions in indexeddb before migrating
[\matrix-org#585](matrix-org#585)
* Emit an event for crypto store migration
[\matrix-org#586](matrix-org#586)
* Supporting fixes For making UnknownDeviceDialog not pop up automatically
[\matrix-org#575](matrix-org#575)
* Move sessions to the crypto store
[\matrix-org#584](matrix-org#584)
* Change crypto store transaction API
[\matrix-org#582](matrix-org#582)
* Add some missed copyright notices
[\matrix-org#581](matrix-org#581)
* Move Olm account to IndexedDB
[\matrix-org#579](matrix-org#579)
* Fix logging of DecryptionErrors to be more useful
[\matrix-org#580](matrix-org#580)
* [BREAKING] Change the behaviour of the unverfied devices blacklist flag
[\matrix-org#568](matrix-org#568)
* Support set_presence=offline for syncing
[\matrix-org#557](matrix-org#557)
* Consider cases where the sender may not redact their own event
[\matrix-org#556](matrix-org#556)
krombel added a commit to krombel/matrix-js-sdk that referenced this pull request Apr 18, 2018
[Full Changelog](matrix-org/matrix-js-sdk@v0.9.2...v0.10.0-rc.1)
* Fix duplicated state events in timeline from peek
[\matrix-org#630](matrix-org#630)
* Create indexeddb worker when starting the store
[\matrix-org#627](matrix-org#627)
* Fix indexeddb logging
[\matrix-org#626](matrix-org#626)
* Don't do /keys/changes on incremental sync
[\matrix-org#625](matrix-org#625)
* Don't mark devicelist dirty unnecessarily
[\matrix-org#623](matrix-org#623)
* Cache the joined member count for a room state
[\matrix-org#619](matrix-org#619)
* Fix JS doc
[\matrix-org#618](matrix-org#618)
* Precompute push actions for state events
[\matrix-org#617](matrix-org#617)
* Fix bug where global "Never send to unverified..." is ignored
[\matrix-org#616](matrix-org#616)
* Intern legacy top-level 'membership' field
[\matrix-org#615](matrix-org#615)
* Don't synthesize RR for m.room.redaction as causes the RR to go missing.
[\matrix-org#598](matrix-org#598)
* Make Events create Dates on demand
[\matrix-org#613](matrix-org#613)
* Stop cloning events when adding to state
[\matrix-org#612](matrix-org#612)
* De-dup code: use the initialiseState function
[\matrix-org#611](matrix-org#611)
* Create sentinel members on-demand
[\matrix-org#610](matrix-org#610)
* Some more doc on how sentinels work
[\matrix-org#609](matrix-org#609)
* Migrate room encryption store to crypto store
[\matrix-org#597](matrix-org#597)
* add parameter to getIdentityServerUrl to strip the protocol for invites
[\matrix-org#600](matrix-org#600)
* Move Device Tracking Data to Crypto Store
[\matrix-org#594](matrix-org#594)
* Optimise pushprocessor
[\matrix-org#591](matrix-org#591)
* Set event error before emitting
[\matrix-org#592](matrix-org#592)
* Add event type for stickers [WIP]
[\matrix-org#590](matrix-org#590)
* Migrate inbound sessions to cryptostore
[\matrix-org#587](matrix-org#587)
* Disambiguate names if they contain an mxid
[\matrix-org#588](matrix-org#588)
* Check for sessions in indexeddb before migrating
[\matrix-org#585](matrix-org#585)
* Emit an event for crypto store migration
[\matrix-org#586](matrix-org#586)
* Supporting fixes For making UnknownDeviceDialog not pop up automatically
[\matrix-org#575](matrix-org#575)
* Move sessions to the crypto store
[\matrix-org#584](matrix-org#584)
* Change crypto store transaction API
[\matrix-org#582](matrix-org#582)
* Add some missed copyright notices
[\matrix-org#581](matrix-org#581)
* Move Olm account to IndexedDB
[\matrix-org#579](matrix-org#579)
* Fix logging of DecryptionErrors to be more useful
[\matrix-org#580](matrix-org#580)
* [BREAKING] Change the behaviour of the unverfied devices blacklist flag
[\matrix-org#568](matrix-org#568)
* Support set_presence=offline for syncing
[\matrix-org#557](matrix-org#557)
* Consider cases where the sender may not redact their own event
[\matrix-org#556](matrix-org#556)
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