-
Notifications
You must be signed in to change notification settings - Fork 23
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
1st commit of remote control widget feature :) #20
Conversation
}) | ||
|
||
//Now that we have a remote control widget interface, perform the same ajax post operations as the default web interface buttons | ||
$(document).bind('click', '#remote-control nav', function(e){ |
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.
You should use on
instead of bind
here, to be consistent.
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.
Okay. Thanks will change it.
Just out of curiosity, is there any difference between 'on' and 'bind' methods? My guess is that is there performance difference use either or of the methods?
This is now done, @sunny. I haven't heard anything back from Valentin in regard of his CSS code. I sent another email to him today and see where he's add with it. If still no reply within 2 weeks, does that mean we can freely use the code? Or do I need to create our own? |
I think you should create your own CSS and HTML for it. Anyway I think it needs icons (by Font Awesome for example) for each button to make it clear what it does. For now I think it is a bit too obscure what each part does. Also, space is not a good shortcut because it plays/pauses at the same time. |
Thanks @sunny. BTW, I got the email reply from Valentin. He's okay with the go-ahead of having me to use his CSS code freely, and I can modify it whatever I like in seeing this feature crafted inside the so-nice repo. |
@sunny - Font-awesome icons are supplied (via CDN). And I've completed music player icons on the remote control widget. Let me know what you think of it when you have the chance. |
Font-awesome is now removed from Gemfile.lock. Cheers! |
… on the remote control widget
Hi @sunny - any chance this feature request to be merge to the master branch? Or do you have any further questions about my setup? Thanks |
Hi @awongCM, sorry for not responding earlier. First of all: awesome job! In fact I like that player so much that I think it should be the default. Before that can happen, though I think we should see the following changes:
What do you think? |
Hey @sunny. No worries about it. Those are great ideas of yours. I love all them! :)
It's interesting to note that you mentioned drag-and-drop feature as I was experimenting that feature on my local dev using Javascript mouse's positioning on screen earlier. Your thoughts are exactly what I thought. :)
Are you hinting that I should make this as mobile responsive? On IPhones or IPads?
Agree. Thought I have been given some thought of making a better css for the remote control widget as per your previous comment. Something like this. Of course, this proposed thought may seem overzealous to do. But that doesn't stop from inspiring ourselves to make such cool widget interface like so?
Are you suggesting the other default control buttons can be made defunct? Is that your suggestion?
Couldn't agree more. |
I think that it should at least be usable on smaller devices (or smaller browser sizes) instead of being hidden like it is now.
Yeah, I agree that it might be a bit too much on that screenshot, but it could be inspiring.
Absolutely. We should only have one button for every function. |
Great. I'm with you on that.
Cool bananas. Let's make this work. |
…is now the default player control option
Hi @sunny - Just letting you know I commited my changes. Let me know what else you think I may missed. Thanks. |
Good job! I'll try and look at this in detail when I get the time. Here is a list of checks to look for:
|
Thanks @sunny. I'll take look at this checklist. And I'll come back to you if I have further questions on them. This is gonna be so exciting to test out! :) |
Hey @sunny. I'm just catching up on this. I got all main items ticked and checked. One question I want to clarify "Fits in small browsers by default" Does this suggest when running so-nice, the browser's dimensions come up being smaller resolutions in width and height? Is that the default behaviour you're talking about? |
I simply meant that it should look good on small screens (whatever the resolution or browser size) |
Thanks @sunny |
I recommend to use the developer tools with Chrome and in device mode to test at all kinds of sizes and to emulate touch. When it all looks good there you can do a final check on your mobile by using some kind of port forwarder like ngrok |
Cheers @sunny! Will check them and let you know if I have questions. |
…smartphone devices(Iphone and Android first)
Hey @sunny I developed initial stages of mobile responsive part of the remote control widget. I tested them on both the actual Chrome browser settings, and using ngrok (which btw is an excellent tool for testing localhost web app on mobile devices!! Wish I knew about this long while ago when I first developed other mobile apps in my previous projects!) I only completed the mobile web browser experience on smartphones, ie Android and Iphone devices - but not for tabular or larger screen devices as of yet. Which is why if you look into my commits changes, I laid out the proper foundation on all possible media queries for all types of device dimensions. I've yet to work on implement the draggable features on mobile devices by using touch simulation. I've looked at a number of 3rd party javascript plugins in the open source community, but none of them are good enough to fit well with our jquery plugins so I'm thinking of writing my own version of touch-drag simulation myself, which I need to spend a little bit of time looking at jquery/HTML5 api how to accomplish this. Hopefully it's not difficult to implement. Let me know if this is a good start. I'll look forward to hearing your comments shortly. Cheers! |
I don't think you should try to list all possible screens… because there are actually way more device possibilities than you have listed! And the list of possible sizes keeps growing. That's why I think you should remove the list of possible devices and just insert a media query whenever the the design breaks instead of aiming for iPad, etc. Check out this article for more details about why I'm saying. For dragging it's ok if it's an HTML5 component as long as it works. You could also use jQuery ui. Keep up the good job! |
Good point @sunny. I think the huge pool of mobile/tablet devices is infinitely growing in consumer electronic space, and it would be foolish to list all media queries for all of them. However, I reserved the thought of laying out the common household devices most people use namely IPad, IPhone, Android as I'm used to writing media queries by hand in my previous mobile web site projects. But thanks for sharing the link. I'd love to take a fresher perspective look how better media queries are done nowadays. Thanks for the handy tips again! |
@@ -116,6 +116,19 @@ $(function() { | |||
}) | |||
} | |||
|
|||
// Toggle the playpause button on the remote button widget | |||
var togglePlayPause = function(toggle_playpause_button){ |
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.
Identifier 'toggle_playpause_button' is not in camel case.
Too many errors. (53% scanned).
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.
Really???
But I couldn't find an existing code anywhere in the script that demonstrates good camel-cased letters.
Would you be expecting something like 'togglePlayPauseButton', perhaps?
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.
@awongCM : houndci is an automated tool that tries to enforce a common style. Don't pay too much attention to what it says since it wasn't there from the start of this project (hound recently added JavaScript, too).
I would be better if it was togglePlayPauseButton
but I'll gladly pull this is as it is, I won't be very fussy about this code style for this project.
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.
Oh I see. Thanks @sunny.
That clears things up.
I'll change it anyways. So that I build up good habits of writing good coding practices for open source projects.
Cheers!
…the old markup content for remote control;
@@ -116,6 +116,19 @@ $(function() { | |||
}) | |||
} | |||
|
|||
// Toggle the playpause button on the remote button widget | |||
var togglePlayPause = function(toggle_playpause_button){ |
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.
Identifier 'toggle_playpause_button' is not in camel case.
Too many errors. (49% scanned).
@sunny - Hey Sunny. It's been a while. I just pushed my changes. Think I have done everything right, though my HTML5 dragging component seems a bit 'fiddly'. Let me know what you think when you have the chance. Cheers. |
If you drag it by holding the left side, the "ghost" (the greyed-out copy that follows the cursor) is 176px too far right from the mouse, it looks weird. (Tested under Chrome.) On a smartphone or computer with touch it seems that it doesn't respond to touch events. Also, I managed to lose the remote when I changed screen resolution and the browser's window got smaller. That is due to the fact that it is draggable and therefore placed in an absolute position, I guess. |
…rs; CSS positioning for the right element when dragging/clicking
@@ -116,6 +116,19 @@ $(function() { | |||
}) | |||
} | |||
|
|||
// Toggle the playpause button on the remote button widget | |||
var togglePlayPause = function(toggle_playpause_button){ |
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.
Identifier 'toggle_playpause_button' is not in camel case.
Too many errors. (51% scanned).
Closing this PR since this is over 4 years old. Feel free to reopen if you want to resolve the conflicts and suggest something new. In any way thank you for participating in the project! \o/ |
HI @sunny. I was reviewing my Github repos moments ago. I had no idea this was an open PR for a long time! Sorry I didn't get to finish off what I started as a hobbyist project at the time. Guess my work life has taken me different direction and interests in other programming interests, most notably Python and NodeJS. 😅 It's been super fun nevertheless. Hope I'll see you around in Github community a bit. :) All the best! |
I added another button to the existing web interface controls.
It's next to the Vote button.
Basically this button will toggle a nice tiny remote control widget appearing to the bottom right of the screen, using some pretty css styling.
It looks like a mini ipod interface with five buttons on it. The fifth button is the middle button, which plays/pause your music. The top two corner buttons are navigating to the next track and volume up buttons respectively; whilst the bottom tow corner buttons are navigating to the previous tack and volume down buttons respectively.
It's a simple initial of remote control ui widget to begin with. Later on, I will decide how to include some icons on the buttons and add a little animation on them when playing, along with other features I could think of such as make the remote control drag-and-drop-able, if that makes sense.
Let me know what you think. :)