Closed Burstaholic closed 8 years ago
@Burstaholic first of all you rock! Thanks a ton for doing this and opening a PR.
@RocketChat/core check this out! Let's see what we can do to help out and get this awesome change merged
I took some time and worked out how to work around SweetAlert's limitations and build a simple <select>
input - I think it works a lot better.
Awesome
@Burstaholic is this working? Can you past a screenshot?
@rodrigok I image that if @Burstaholic submitted a PR he expect this to be working :smile:
But I don't have an Android and would also love a screenshot :+1:
@engelgabriel as far as I know this line will not work at the client side https://github.com/Burstaholic/Rocket.Chat.Cordova/blob/burstaholic-easy-share/www/shared/js/share.js?ts=2#L9
RocketChat.models are only available at the server side. So I'm curious how this is working.
@rodrigok I'm new to Meteor, but that works fine client-side, even in the console in the regular desktop web page.
In fact I'm eager for https://github.com/RocketChat/Rocket.Chat/issues/1367 to be resolved, because I uploaded so many copies of the same picture while testing this :tongue:
I'll get some screenshots for you - I did most of this with the Android emulator so I wouldn't mess up the app on my phone. It's pretty good now that it supports hardware virtualization.
In the process of taking these, I found a bug and fixed it - because Android Studio always starts the app, I didn't realize there was a timing issue with setting up the share handler when the app was not already running.
Also I changed the code on lines 12 & 29 to hopefully be more in line with the way your model objects are intended to be used (based on collections.coffee
).
Edit - shot of the room selection:
@Burstaholic A couple of suggestions on the room selection:
#
, @
, or :lock: in front of the room to signify which type it is.@Burstaholic fantastic! :smile:
Put either #, @, or :lock: in front of the room to signify which type it is. Change the sorting: use the lowercase version of the room name group by room type: direct messages, private groups, channels.
If you are able to do this, that would be amazing.
I'm not sure how familiar you are with the rocket.chat room types but if you access the t
property on the rooms you can see the room type to accomplish this.
Also might reduce the number of rooms returned to only the ones you have active by changing your ChatSubscription.find
to something like:
_.sortBy(ChatSubscription.find({open: true}).fetch(), 'name');
Because if anything like me I have several hundred on the demo server, that'd take a long time to find the room I wanted to send to. :grin:
Ok, I like these ideas.
This will sort the types in the same order they appear in the sidebar (see below). I can also make a map like var sortOrder = { d: 1, p: 2, c: 3 };
to create a different order.
var rooms = _.sortBy(ChatSubscription.find({ open: true }).fetch(), function(room)
{
return [room.t, room.name.toLowerCase()].join('-');
});
Unfortunately HTML is not allowed in <option>
elements, so Private Groups will need a character prefix or none at all (not sure if UTF could be used).
@Burstaholic so you could map:
@
&
#
I think just the character alone is fine
I got a UTF padlock character to work, but Android displayed it as a colored emoji so it looked a bit jarring. Here's what I have now:
@Burstaholic That looks much nicer! Sweet!
@burstaholic that looks much better!
It was actually kind of bothering me having them out of order compared to the sidebar, so I put it that way (which also makes the sort simpler).
If you guys are OK with that, I like this better.
I haven't tested this, but how does it work when the user has more than one server? What if I want to send it to another server which isn't my last active one?
That's a good point - there should be another select for servers. I'll add that in.
FYI, I'm still working on this. The combination of the cordova-sharingreceptor
plugin and the Crosswalk WebView have trouble correctly handling the page navigation when the app switches to a new server.
I've submitted bug reports to both projects, and I'll see what they say. At worst, I can fork cordova-sharingreceptor
and make a small modification that will fix the issue, then publish that version. Hopefully the author will be responsive and that won't be necessary :)
Details for the curious at:
Thanks for the feedback @Burstaholic
If we accept it as it is, could it at least work for single servers users?
Yes - as long as your current active server is the one you want, it works fine. Also, you can cancel out of the room choice dialog if you realize you need to switch servers.
This adds the cordova-sharingreceptor plugin and a basic SweetAlert input to select which room to share to.
The main thing it needs is a more sophisticated UI - suggestions on the best way to build that here are welcome.