Skip to content
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

Ignore special tracks for completeness #383

Merged
merged 8 commits into from
Feb 8, 2015

Conversation

phw
Copy link
Member

@phw phw commented Dec 4, 2014

This adds an option to ignore some track types for determining the completeness of an album. This makes it easier for a user to see completed albums and ignore tracks she doesn't care about. Currently the user can choose to ignore:

  • Video tracks
  • Data tracks
  • Pregap tracks
  • Silent tracks

In addition this PR adds some more changes to simplify the work with those tracks:

  1. Video and data tracks get a different icon instead of the default musical note icon (in case a track is both a data and a video track the video icon gets precedence)
  2. Adds a $is_complete() tagger script function
  3. Sets a ~video tagger script variable for video tracks
  4. Ignored tracks are displayed grayed out, so they are still available for tagging but not as prominent in the listing

This PR will fix PICARD-514 and PICARD-652

This PR is not ready for merging due to the new icons. I have added icons from the Tango icon set for the track types because this is the icon set the current note icon is taken from. But I think the icons are a) really ugly and b) make the UI way too noisy. But since I am not very talented in designing icons and I am not sure when I will find the time to look for nicer icons with an acceptable license I just publish the code here and ask you all for help :)

Some notes about the $is_complete() function: This is somewhat similar to the $isperfectalbum function @Sophist-UK proposed in PICARD-637. But I deliberately decided not to add the special logic of having only one matched file per track. I think this is a different change which should be discussed separately. For me it was important that $is_complete() and the visual representation of the completeness are identical.

…eness.

Currently this will ignore video, pregap, data and silent tracks.
Fixes PICARD-514
Works currently only for renaming scripts (similar to $matchedtracks). PICARD-514
…ctly under the input fields.

This was accidentally moved when adding new fields to this options page.
@phw
Copy link
Member Author

phw commented Dec 4, 2014

Another issue is the wording in the options dialog. Currently it reads "Ignore the following tracks for release completeness". Is this proper English and is it clear what it means?

@phw
Copy link
Member Author

phw commented Dec 4, 2014

This is also related to PICARD-185, but does not solve the main issue there (ignoring data tracks for numbering). I could extend this patch to also have ignored track types not to be used for the numbering, but I'd prefer this to be done in a separate PR after this one has been merged.

@96187
Copy link

96187 commented Dec 4, 2014

I would probably say "when determining whether a release is complete" rather than "for release completeness", or at least "when determining release completeness" if nothing else. What you have doesn't sound wrong, as such, I just think it's not necessarily very clear if you don't already know what it's referring to.

Could you take a screenshot of how it looks with the icons?

@phw
Copy link
Member Author

phw commented Dec 4, 2014

@96187 Thanks for the feedback. Here is a screenshot of how the icons look like. This also shows that in this case the data tracks are ignored when determining whether a release is complete, but not the pre-gap track (but it could be).

picard-track-types

The following screenshot shows the options screen:

picard-track-type-options

@96187
Copy link

96187 commented Dec 5, 2014

Thanks for the screenshot. :)

The cog icon looks weird to me, I would use a file icon of some sort, because data tracks which aren't videos are mostly going to be things like MP3 files, so a cog doesn't have any obvious connection.

The video icon doesn't look too bad to me, although it might blend in better if it used the same colours as the music note, which might be simple enough to do?

@mineo
Copy link
Member

mineo commented Dec 5, 2014

The code looks good to me, but I'm not going to be of any help with graphics stuff :p

@zas
Copy link
Collaborator

zas commented Dec 7, 2014

What about something like :
track-data
for data track ?

@@ -602,6 +602,13 @@ def func_matchedtracks(parser, arg):
return "0"


def func_is_complete(parser):
if parser.file:
if parser.file.parent and parser.file.parent.album.is_complete():
Copy link
Collaborator

Choose a reason for hiding this comment

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

A single if + and would be enough here

@zas
Copy link
Collaborator

zas commented Dec 9, 2014

Overall the patch is ok for me, nice addition imho.

@phw
Copy link
Member Author

phw commented Dec 9, 2014

Ok, I will take all your feedback into consideration and update this PR (I hope sometime before the holidays).

@phw
Copy link
Member Author

phw commented Jan 26, 2015

I have played with the icons, please give me some feedback.

New icons, less colorful:
bildschirmfoto vom 2015-01-26 22 53 40

Only the data icon track replaced:
bildschirmfoto vom 2015-01-26 22 59 56

For comparisson the old screenshot from above:
picard-track-types

Icons use symbols from Font Awesome by Dave Gandy - http://fontawesome.io
@zas
Copy link
Collaborator

zas commented Jan 27, 2015

I like those in

@zas
Copy link
Collaborator

zas commented Jan 31, 2015

@phw : i'm ok to merge it once you'll addressed following comments
#383 (comment)
#383 (comment)

@phw
Copy link
Member Author

phw commented Feb 1, 2015

@zas: I've updated the code

@zas
Copy link
Collaborator

zas commented Feb 1, 2015

We still need more feedback about new icons, but for me it is ready for merge.

@reosarevok
Copy link
Member

I prefer the black and white icons in isolation, but dunno how well they work with the rest of the interface. Any of the two options is an improvement though, so both seem good enough.

@phw
Copy link
Member Author

phw commented Feb 4, 2015

Just because I didn't say this: The black icons are also what is currently in this PR as they are the ones I prefer, too :)

@ianmcorvidae
Copy link
Contributor

The black-and-white don't seem to match the style of the rest of the interface to me, especially the disc icon, or if not changed the unmatched file icon which currently uses the color notes. A mix of flat and 3D icons seems weird, not to mention a mix of color and non-color, and the black-and-white/flat looks very pre-internet-age-style, or something, to me. The video one maybe contributes to that since really, nobody's using this to print labels for film canisters (and the screen icon matches what's used on mb.org better as well, which may or may not be a goal). So I'd vote for the newer color icons, perhaps ideally with a less flat "binary written on paper" type icon. I do think that's better than the cog for denoting data track, though.

For consideration/comparison, mb.org uses this for data tracks: data track icon

@Freso
Copy link
Member

Freso commented Feb 4, 2015

+1 to what @ianmcorvidae said.

@phw
Copy link
Member Author

phw commented Feb 6, 2015

I see the issues you have with the flat icons. Well, actually not all :) I don't think these icons look "pre-internet-age-style", rather they are the current popular icon style on many web pages. The colored Tango icons Picard uses are so 90s :P But the real reason I wanted to try those is that they remove visual clutter from the listings and make the main information stand out more while still providing a clear distinction for the track types.

Anyway, when using the data icon from MB this looks like this:

bildschirmfoto vom 2015-02-06 20 51 12

This is probably the version most consistent with both Picards original style and the MB website. I have not pushed those changes yet, but if feedback is positive I will push it and merge this PR.

@ianmcorvidae
Copy link
Contributor

I'd maybe support the black-and-white if they were greys rather than pure black-and-white; that's really the thing that makes it feel like I'm looking at Windows 3.1 to me :)

In any case, I like the version with the data track icon from the site if others are fine with that. Perhaps we can move to a flatter icon style in another PR if that's desired, and keep it consistent here.

@96187
Copy link

96187 commented Feb 6, 2015

I agree with what reosarevok said. I like the black and white icons, at least in the screenshots here.

Having black and white base icons also leaves open the possibility of adding coloured variants for things like matched tracks, tracks with errors, etc in the future (probably with ticks or exclamation marks so that it's not just colour).

The only icon I really didn't like though was the cog.

@zas
Copy link
Collaborator

zas commented Feb 7, 2015

Having black and white base icons also leaves open the possibility of adding coloured variants

Good point. It may be useful in the future.

that's really the thing that makes it feel like I'm looking at Windows 3.1 to me :)

Windows 3.1 had colorful icons.

phw added a commit that referenced this pull request Feb 8, 2015
@phw phw merged commit ea1af8a into metabrainz:master Feb 8, 2015
@phw
Copy link
Member Author

phw commented Feb 8, 2015

Since the consensus seems to be, that the colored icons are ok and more consistent with current Picard, but the flat icons might be nice to reconsider later, I have merged this PR together with the colored icons. The flat icons are still in the commit history, so if somebody wants to build on this please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants