Added queue position and approx wait time. #402 #404

SuhanKoh · 2017-11-27T20:16:04Z

No description provided.

Shredder121 · 2017-11-27T23:58:51Z

IIRC embeds didn't look as nice.
Or was it decided to do that?
(Might have missed it)

Abi45x · 2017-11-27T23:59:47Z

We decided against embeds because they don't look as nice as the current queue

SuhanKoh · 2017-11-28T00:00:13Z

I have screenshots here, #402

Shredder121 · 2017-11-28T00:06:13Z

Ok I see napster said it's okay

SuhanKoh · 2017-11-28T00:09:07Z

Is there another way to display long message without using embeds? The issue here is once I added queue position and wait timer, the message gets too long :/

Shredder121 · 2017-11-28T06:58:46Z

Without embeds there is more potential screen real estate (on desktop clients*)
embeds only grow as big as they do, so yeah..

schnapster · 2017-11-28T07:03:22Z

Embeds use a smaller font.

schnapster · 2017-12-12T11:54:15Z

Hi, can you provide screenshots please?

SuhanKoh · 2017-12-18T05:29:44Z

SuhanKoh · 2017-12-18T05:33:10Z

Squeezed to smallest possible on a macosx.

freyacodes

FredBoat shouldn't use embeds where it isn't useful. Not all users see them and it is inconsistent with our other messages.

schnapster · 2017-12-21T16:56:42Z

FredBoat shouldn't use embeds where it isn't useful. Not all users see them and it is inconsistent with our other messages.

I'd like to hear your counterproposal on displaying a lot of information without bloating the messages.

freyacodes · 2017-12-21T16:59:11Z

I'd like to hear your counterproposal on displaying a lot of information without bloating the messages.

With a normal message, possibly less verbose than the current one

SuhanKoh · 2018-01-02T05:06:58Z

Should I concat the music name to maybe to x characters and replace it with "..." ?

SuhanKoh · 2018-01-02T05:24:10Z

How does this look?

schnapster · 2018-01-08T22:36:59Z

@Frederikam 👀

freyacodes · 2018-01-08T22:40:45Z

Instead of commas I would put linebreaks

schnapster · 2018-01-08T22:46:58Z

I think there seem to be equivalent but shorter replacements for most strings:
"has been seletected" -> "selected"
"position in queue" -> "position", or maybe just "queued"
"approximate wait time" -> "~ wait time", or maybe "playing in"

The last "minute" is completely wrong..can probably be replaced by single letter notation (h, m, s). We might have methods for that in TextUtils already.

SuhanKoh · 2018-01-09T02:59:38Z

@Frederikam I think if I follow what napstr suggested, we don't need a line break. I will put up screenshots soon^tm.

SuhanKoh · 2018-01-09T03:03:16Z

@napstr I looked at TextUtils, is this the method you're referring to? TextUtils#formatTime

SuhanKoh · 2018-01-09T03:16:34Z

Any suggestion on how to put line break?

Maybe something like this?

Song selected #2 ~ Now playing
    HELLO! Songs Jukebox | Akhil Akkineni, Kalyani Priyadarshan | Vikram K Kumar | Anup Rubens (29:49) 

Song selected #1 ~ queued: #2, playing in: 29:49
    Adele - Hello (06:07)

freyacodes · 2018-01-09T10:15:54Z

You're right, that actually looks better

I haven't reviewed the code, but I think Napster wanted to do that

schnapster · 2018-01-09T15:09:36Z

I like the song name on the second line too.

Nanabell · 2018-01-09T18:12:05Z

I have a question. How are Livestreams handled here ?

SuhanKoh · 2018-01-10T06:01:06Z

@Nanabell It will now only print once for livestream. Seems like the one in production will print twice.
This PR will only change the formatting of the search/selection result.

SuhanKoh · 2018-01-10T06:03:25Z

Screenshot for the current change:

schnapster · 2018-02-08T01:46:59Z

Would be nice to find a way to incorporate them. Like, usually we show the name of the playlist and the amount of songs on it, instead of the songname and it's length.

schnapster · 2018-02-09T21:25:45Z

Is this waiting on further review? Or is the code still WIP since the last one?

SuhanKoh · 2018-02-11T06:51:02Z

I have updated the code, sorry for the wait.
In this commit, I have made some assumption, so please let me know if this is ok.

When playing from playlist, and hastebin. I've set a max 5 queue messages, then print the counter, which is same as before.

SuhanKoh · 2018-02-11T07:10:28Z

Screenshot for split

schnapster · 2018-02-12T21:24:37Z

Let's reduce it to 3 songs and a clue that more have been added like a line of

...
between the details and the total count at the bottom?

@Frederikam needs to okay this.

…a "..." line between the detail.

SuhanKoh · 2018-02-13T03:25:01Z

@Frederikam @napstr Let me know if this is appropriate. I added it because in the production, it usually reply to playlist with "Added 49 tracks. Found too many tracks to display." due to the > 800 characters check.

freyacodes · 2018-02-26T09:04:06Z

Sorry for the very late response. I haven't followed the conversation, so can you give me a summary of what changed? Is this the only thing that was changed?

Shredder121 · 2018-02-26T10:23:52Z

For trackLoaded it's also using the new format, so maybe also make a screenshot with loading a single track?
Then we can decide if it's what we would distribute to the masses.
Since I agree having a compact list of tracks, but I'd like to be sure the format if right for single tracks as well.

Does it also use the new format with searches? Just wondering.
(Also sorry for hijacking the review Fred 👀 you can continue)

SuhanKoh · 2018-02-27T02:32:41Z

@Shredder121

I loaded the same song to show how it looks like loading single track with empty queue and non-empty queue.
Song selected via search will also use the format. Minus the "Found and added x songs..." message.

SuhanKoh · 2018-02-27T02:34:33Z

@Frederikam What was changed last time you reviewed was performing the commands below will also use the new printout, with queue and x time to play. And removed embeds and use the text format instead.

as a youtube playlist
as a spotify playlist
as a hastebin link
via ;;split

SuhanKoh · 2018-03-15T05:30:05Z

@Frederikam I think I have made all your request changes.

freyacodes · 2018-03-15T14:48:41Z

Unfortunately I don't feel like I have time to review this anytime soon, but at any rate, you should probably resolve those merge conflicts.

schnapster · 2018-03-15T14:50:21Z

Whats the point in solving merge conflicts over and over when the bottleneck is that there are no clear guidelines to the UI/UX of this?

freyacodes · 2018-03-15T14:57:09Z

@napstr I don't believe I have as strong opinions about UX as you think I do. If you want you can review and merge this. As mentioned I haven't really followed the conversation.

Out of date

freyacodes · 2018-03-15T15:01:09Z

FredBoat/src/main/resources/lang/en_US.properties

-selectSuccess=Song **\#{0}** has been selected\: **{1}** ({2})
+selectSuccess=Song selected {0}
+selectSuccessPartNowPlaying=Now playing
+selectSuccessPartQueueWaitTime=Queued: {0}, playing in: {1}.


Though it may make more sense to say "Queued as #{0}"

schnapster · 2018-03-15T15:15:27Z

I don't want to review or manage anything UI/UX related.

* dev: (178 commits) Convert to reactive web app Replace tomcat with undertow, remove unused jpa dependency Replace Spark API with Spring servlet based API Replace a few static calls to Launcher with injected dependencies Make YoutubeAPI a Component Make PlayerLimiter a Component Fix bot admins not recognized in private messages Add monocrome svg Resolve a few warnings and typos Snakeyaml plz Use gradle's new junit support Bump gradle to v4.6 Bump dependencies Remove obsolete tunnel configuration Turn off ehcache overflow to disk Adjust quarterdeck backend naming Backend has its own repository Handle uncaught errors more user friendly Refactor Sentry into Configuration, add mdc tags to it Add logs volume for backend service ... # Conflicts: # FredBoat/src/main/java/fredboat/audio/queue/AudioLoader.java # FredBoat/src/main/java/fredboat/command/music/control/SelectCommand.java # FredBoat/src/main/java/fredboat/util/TextUtils.java

Shredder121

I think it looks fine, we can polish later if need be.

NOTE: I am not quite content with patching i18n'd strings together, will have to look and see if it generates problems for translators/output to users.

SuhanKoh · 2018-07-29T16:58:32Z

I don't think there are interest in merging this pr, so I will go ahead and close it

freyacodes · 2018-07-30T20:42:16Z

@SuhanKoh the problem is that the sentinel branch is blocking this PR. Much of this code would probably need to be changed, and I might do that migration once we deploy the next major version of FredBoat.

This PR may not be merged, but I am keeping it open just so I won't forget.

Added queue position and approx wait time. freyacodes#402

db9d3f0

freyacodes previously requested changes Dec 21, 2017

View reviewed changes

Removed selection embed.

8d853a5

Changes on remaining time formatting and string wording.

b3bc027

Modified selection string to be two lines.

661d6d5

Added queue message to playlist, and fix timer for direct url.

dcde4c8

SuhanKoh added 2 commits February 10, 2018 23:01

Added limit check for split command.

f92647a

Added a check for ignoring empty or null title on split command reply.

a06fa87

Reduced max queue message display for split and load playlist. Added …

59f7ca2

…a "..." line between the detail.

schnapster added the UX label Mar 15, 2018

freyacodes reviewed Mar 15, 2018

View reviewed changes

SuhanKoh added 2 commits March 15, 2018 21:17

Updated selection queue message.

b462449

Shredder121 approved these changes Mar 16, 2018

View reviewed changes

SuhanKoh closed this Jul 29, 2018

freyacodes reopened this Jul 30, 2018

freyacodes added the stalled-pr Candidate for closure label Feb 9, 2019

Added queue position and approx wait time. #402 #404

Are you sure you want to change the base?

Added queue position and approx wait time. #402 #404

Conversation

SuhanKoh commented Nov 27, 2017

Shredder121 commented Nov 27, 2017

Abi45x commented Nov 27, 2017

SuhanKoh commented Nov 28, 2017

Shredder121 commented Nov 28, 2017

SuhanKoh commented Nov 28, 2017

Shredder121 commented Nov 28, 2017

schnapster commented Nov 28, 2017

schnapster commented Dec 12, 2017

SuhanKoh commented Dec 18, 2017

SuhanKoh commented Dec 18, 2017

freyacodes left a comment

Choose a reason for hiding this comment

schnapster commented Dec 21, 2017

freyacodes commented Dec 21, 2017

SuhanKoh commented Jan 2, 2018

SuhanKoh commented Jan 2, 2018

schnapster commented Jan 8, 2018

freyacodes commented Jan 8, 2018

schnapster commented Jan 8, 2018 • edited Loading

SuhanKoh commented Jan 9, 2018

SuhanKoh commented Jan 9, 2018

SuhanKoh commented Jan 9, 2018

freyacodes commented Jan 9, 2018

schnapster commented Jan 9, 2018

Nanabell commented Jan 9, 2018

SuhanKoh commented Jan 10, 2018 • edited Loading

SuhanKoh commented Jan 10, 2018 • edited Loading

schnapster commented Feb 8, 2018

schnapster commented Feb 9, 2018

SuhanKoh commented Feb 11, 2018 • edited Loading

SuhanKoh commented Feb 11, 2018

schnapster commented Feb 12, 2018

SuhanKoh commented Feb 13, 2018

freyacodes commented Feb 26, 2018

Shredder121 commented Feb 26, 2018

SuhanKoh commented Feb 27, 2018 • edited Loading

SuhanKoh commented Feb 27, 2018 • edited Loading

SuhanKoh commented Mar 15, 2018

freyacodes commented Mar 15, 2018

schnapster commented Mar 15, 2018

freyacodes commented Mar 15, 2018

freyacodes Mar 15, 2018

Choose a reason for hiding this comment

schnapster commented Mar 15, 2018

Shredder121 left a comment

Choose a reason for hiding this comment

SuhanKoh commented Jul 29, 2018

freyacodes commented Jul 30, 2018

schnapster commented Jan 8, 2018 •

edited

Loading

SuhanKoh commented Jan 10, 2018 •

edited

Loading

SuhanKoh commented Jan 10, 2018 •

edited

Loading

SuhanKoh commented Feb 11, 2018 •

edited

Loading

SuhanKoh commented Feb 27, 2018 •

edited

Loading

SuhanKoh commented Feb 27, 2018 •

edited

Loading