search

pencil / rechat

Adds chat messages from the past to your favorite Twitch VODs
https://www.rechat.org/
MIT License
66 stars 11 forks source link

Added ability to manually adjust chat delay #30

Open andairo opened 9 years ago

andairo commented 9 years ago

This pull request adds some additional ui elements along with the ability to manually adjust the chat delay.

The delay is updated by modifying the existing variable ReChat.Playback.streamDelay based on user input.

Repurposed the familiar twitch chat room selection button to show/hide the delay adjustment controls. Here is the header with controls toggled off: rechat1

toggled on: rechat2

The dropdown changes how many seconds the delay is adjusted by on each operation. The left button decreases the delay by the selected amount and the right button increases the delay by the selected amount. The display shows the current delay offset.

Tested in Chrome 45.0.2454.101 m and Firefox 41.0.1

All feedback is appreciated, thanks.

goldbattle commented 9 years ago

Seems to take up a lot of unneeded space. What about adding the input box on the top? Trying to keep what there is to a minimum. image

andairo commented 9 years ago

Yeah, you're right.

That's actually how I had it initially when I had just done it for personal use but the main reason I did it this way in the end is to streamline what kind of input can be received from the user thus removing the need for input validation or notifying the user about input errors. I did anticipate that noone would want additional space being taken from the chat area which is why i added the toggle button so you could make adjustments then hide the bar when you're done.

If the space is more of a priority then your way is definitely better. If I have some time later after work i'll make some changes, unless you already have?

goldbattle commented 9 years ago

All up to you, just my thoughts. On Oct 12, 2015 8:23 AM, "Andre Blair" notifications@github.com wrote:

Reopened #30 https://github.com/pencil/rechat/pull/30.

— Reply to this email directly or view it on GitHub https://github.com/pencil/rechat/pull/30#event-432879933.

pencil commented 9 years ago

Thanks for your work! I think the UI is not very clear especially for the unexperienced user. Maybe a slider (ranging from 0 to 5 minutes delay) would be a better user experience?

Also the delay settings should be somewhat hidden since the default delay is good enough in 90% of the cases. Similar to how Twitch hides the chat settings behind the image

andairo commented 9 years ago

Thanks for the slider suggestion! I was also worried that the functionality was a bit hard to understand for a first time user but i wasnt sure of an easier way to implement it. That would be much more intuitive I agree. I will work on that tonight since I have the day off work tomorrow.

Im not fully sure what you meant in the second paragraph. The delay settings are currently hidden when the page is loaded and can be shown and hidden by pressing toggleb. Are you suggesting to use settb instead? That seems like the right idea as well (even if thats not what you meant).

goldbattle commented 9 years ago

It would be better to use a settings icon, compared to the group chat icon.

On Sun, Oct 18, 2015 at 4:37 PM, Andre Blair notifications@github.com wrote:

Thanks for the slider suggestion! I was also worried that the functionality was a bit hard to understand for a first time user but i wasnt sure of an easier way to implement it. That would be much more intuitive I agree. I will work on that tonight since I have the day off work tomorrow.

Im not fully sure what you meant in the second paragraph. The delay settings are currently hidden when the page is loaded and can be shown and hidden by pressing [image: toggleb] https://cloud.githubusercontent.com/assets/5339718/10566467/4a6a7c6a-75ad-11e5-8652-dcc2528cd923.PNG. Are you suggesting to use [image: settb] https://cloud.githubusercontent.com/assets/5339718/10566491/d5bbbd4c-75ad-11e5-9cfc-893a371e3287.png instead? That seems like the right idea as well (even if thats not what you meant).

— Reply to this email directly or view it on GitHub https://github.com/pencil/rechat/pull/30#issuecomment-149045126.

andairo commented 9 years ago

Yes I will change that as well. At the time i was focusing too much on making the header look similar to the normal Twich header but not realizing that using the settings icon would make a lot more sense :sweat:

andairo commented 9 years ago

page loaded: pic1

press settings button: pic2

changed delay: pic3

press settings button: pic4

goldbattle commented 9 years ago

It is looking better, I wouldn't have a text input, and instead just have text there, and have the slider be the control scheme. It would look better then having that textbox there (assuming that is what that is)

Also, are you saving this from page to page? It would be best that the user does not have to change this every time when they refresh the page, either local storage or something of that sort could solve that, but you might already have that implemented.

Also, when you are merging to get rid of conflicts, do that in a separate commit from your normal code push. It is tough to wade through and figure out what the programer changed, and what is part of the merge. I wouldn't worry about the amount of commits you have.

pencil commented 9 years ago

I think you accidentally squashed the merge of master with your own commits. Diff looks weird.

andairo commented 9 years ago

Had a super busy week so I didnt have much time to check in but yeah thats my bad, sorry :sweat: I wasnt sure of what was the standard procedure for merging, probly should have asked.

Thanks for the tips @goldbattle, I'll work on it tomorrow then revert my last commit, make a fresh one with only merge code from master, then another commit with my changes.

andairo commented 9 years ago

Made some changes based on suggestions from @goldbattle

Changed the element that displays the delay setting: newlook

It now remembers the setting you selected for a channel. If you havent changed the settings on a particular channel, it will simply use the default delay since as @pencil said the default delay works well in the vast majority of cases.