Skip to content

Print duration of selected tracks in AutoDJ#749

Merged
daschuer merged 3 commits intomixxxdj:masterfrom
idcmp:master
Oct 29, 2015
Merged

Print duration of selected tracks in AutoDJ#749
daschuer merged 3 commits intomixxxdj:masterfrom
idcmp:master

Conversation

@idcmp
Copy link
Copy Markdown

@idcmp idcmp commented Oct 19, 2015

Relates to https://bugs.launchpad.net/mixxx/+bug/1505408 and possibly offers a fix.

Adds a human friendly duration of the selected tracks in AutoDJ next to the "Enable AutoDJ" button:

1 hour, 34 seconds (24 tracks)

The exact time is available as a tooltip.

NOTE: I'm not a C++ programmer, so I may have overlooked something. 😄

I ran lupdate to update the mixxx.ts file. It made a truckload of changes to the file (mostly line number changes), but I've tried to only include the relevant lines in this pull request.

(I'd love it if this could make the next 1.12 beta.)

Comment thread src/dlgautodj.cpp Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some spaces are missing here. Give a look to this example from cachingreader :

for (int i = 0; i < maximumCachingReaderChunksInMemory; ++i) {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess you forgot to push as I see no change here ;)

@jclaveau
Copy link
Copy Markdown

These notes are code nit-picks but it will make @daschuer happy if they are fixed ;)

Thanks for your work.

@idcmp
Copy link
Copy Markdown
Author

idcmp commented Oct 19, 2015

@jclaveau Will update spacing! Is there a formatter I can use going forward?

@jclaveau
Copy link
Copy Markdown

There have been discussions about it : #616
But nothing has been definitely chosen I think.

Simply give a look at this :
http://mixxx.org/wiki/doku.php/coding_guidelines

And you'll see it's not that long to learn.

@daschuer
Copy link
Copy Markdown
Member

Our style is defined here:
http://www.mixxx.org/wiki/doku.php/coding_guidelines

I use Eclipse Autoformater:
http://www.mixxx.org/wiki/doku.php/coding_guidelines#auto-formater
other contributor use clang.

@daschuer
Copy link
Copy Markdown
Member

The position of the new info does not work for netbooks. There is not enough horizontal space left in the tree view.
You can verify it by using the Shade skin without resizing it.

How about generalize this feature for all crates and play-lists.
Currently we have a static info:
"Party (13) 42:25"

once we have some tracks selected it may turn into:
"Party (4) 10:02 selected"

This way all views can benefit from this nice new feature.

@idcmp
Copy link
Copy Markdown
Author

idcmp commented Oct 25, 2015

I've compacted the time being displayed to use @uklotzde's suggestion; this text also now fits in the default size for the Shade skin (for Netbooks).

I generally try to make the left-side playlists/crates/autodj panel only as wide as it needs to be to see the names of the lists, and use the remaining width for the list of tracks, so generalizing the feature and putting in the left panel wouldn't solve my problem.

I'd be open to introducing a full-width line under the existing library UI to add status information (but I appreciate that real estate is precious and rare).

@uklotzde
Copy link
Copy Markdown
Contributor

Nice. To fit into the limited screen space is another good reason for
compactness ;) And of course consistency with existing concepts how to
display a duration.

On 10/25/2015 07:51 AM, Idcmp wrote:

I've compacted the time being displayed to use @uklotzde
https://github.com/uklotzde's suggestion; this text also now fits in the
default size for the Shade skin (for Netbooks).

I generally try to make the left-side playlists/crates/autodj panel only
as wide as it needs to be to see the names of the lists, and use the
remaining width for the list of tracks, so generalizing the feature and
putting in the left panel wouldn't solve my problem.

I'd be open to introducing a full-width line under the existing library UI
to add status information (but I appreciate that real estate is precious
and rare).


Reply to this email directly or view it on GitHub
#749 (comment).

@daschuer
Copy link
Copy Markdown
Member

This version works for me, on my netbook. LGTM Thank you.

Before we can merge this, you need to become a Mixxx contributor.
Please sign: https://docs.google.com/a/mixxx.org/spreadsheet/viewform?formkey=dEpYN2NkVEFnWWQzbkFfM0ZYYUZ5X2c6MQ

And comment here when done. Which name should I put to the Mixxx about box?

There are still some line break issues. If you like, you can verify whether you code uses 4 space indent and { at line end. Otherwise I will fix it after merge.

@idcmp
Copy link
Copy Markdown
Author

idcmp commented Oct 29, 2015

Form signed. If you could put JAmes Atwill in the About box, that would be fun. :)

I would appreciate if you could fix the tiny formatting issues. I set KDevelop to 'spaces', but it kept putting in tabs; I'm going to try something else out in the future.

Thanks for your patience with me in adding this!

@daschuer
Copy link
Copy Markdown
Member

Thank you for working on this.

daschuer added a commit that referenced this pull request Oct 29, 2015
Print duration of selected tracks in AutoDJ
@daschuer daschuer merged commit be8e922 into mixxxdj:master Oct 29, 2015
@esbrandt esbrandt mentioned this pull request Jun 27, 2017
37 tasks
esbrandt added a commit to esbrandt/manual that referenced this pull request Jun 27, 2017
esbrandt added a commit to esbrandt/mixxx that referenced this pull request Jun 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants