-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
allow sending any emoji as reaction #2087
Conversation
44574ce
to
64324ea
Compare
@zeitschlag i meanwhile also think, it would be best to "copy" the project to ours and change the few line that make the whole thing a "bottom sheet" and remove the arrows. this will probably also solve the issue wrt emulator. i will do a commit tomorrow ... |
@r10s That's what I suggested yesterday. I wouldn't copy it, but fork their repository, do the changes there and use SPM/pods to include the forked repo. What do you think? |
64324ea
to
7dc0018
Compare
@zeitschlag thanks for all your tips, i forked the repo and added it via Swift Packet Manager, still getting the emulator-cannot-build error. i tried adding the same via Pod - just the same error: the magic behind that is totally unclear to me ... i mean, the forked repo should not have some artefact, things are just compiled - why does the emulator cause issues? also, why does the emulator asks for rosetta - on an M1 arm64 (which is compiling fine for real device) should be just fine |
7dc0018
to
21b7d47
Compare
thanks @zeitschlag for all the tipps! finally, using Cocoapods and setting iOS target to 12 at deltachat/MCEmojiPicker@d9eb480 did the job - the same for Swift Package Manager did not work and leaves us with unusable emulator this makes this PR huge, however, the first commit is the real change, the second one is the result from |
21b7d47
to
29b8365
Compare
no super-important, but seems to be super-simple - i came over that at #2087 and remembered the request from https://support.delta.chat/t/enlarge-the-account-switcher-in-dc-ios-by-swiping-it-up/2941 :)
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.
LGTM, works better than expected, I have to admit!
thanks a lot for guiding the boat through the rough sea-of-dependency! |
no super-important, but seems to be super-simple - i came over that at #2087 and remembered the request from https://support.delta.chat/t/enlarge-the-account-switcher-in-dc-ios-by-swiping-it-up/2941 :)
currently, random reactions (as added with #2087) cannot be retracted easily. this PR allows retraction by just selecting the emoji again - similar to the predefined reactions. (showing the selection in the emoji selector is out of scope of this PR and probably not worth the effort, at least not now where there are much larger fishes to get)
currently, random reactions (as added with #2087) cannot be retracted easily. this PR allows retraction by just selecting the emoji again - similar to the predefined reactions. (showing the selection in the emoji selector is out of scope of this PR and probably not worth the effort, at least not now where there are much larger fishes to get)
i had a quick look how we can improve reactions without much effort, a big impact will have the option to send any reaction.
i used the "first best" emoji picker, and this PR is the result of not much more than an hour of work, with quite some impact, i'd say :)
EDIT: i spent a little more tome to make the selector a better fitting sheet, for that, it was needed to fork the repo at https://github.com/deltachat/MCEmojiPicker (which is still added via Swift Packet Manager)
(OT, reminder to myself, sth. for another PR: if we have sth. like the emoji picker in, i think, we can also reconsider going for the max-4-horizontal items, "👍 👎 ❤️ •••")