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

Add space-padding to missing tracks dialog #2962

Merged
merged 6 commits into from Jun 20, 2018

Conversation

Projects
None yet
2 participants
@jams2
Copy link
Contributor

commented Jun 19, 2018

beet

Hi all,
during a long session organising my library with Beets, I found readability was not great after a while, this might help.
Not sure how to write tests for this, happy to be pointed in the right direction, and happy to look at other dialogs if this is helpful.

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jun 19, 2018

This looks really great; thanks for looking into it!

Just for the sake of thoroughly considering the alternatives, how about this idea: instead of moving over the right-hand column by a fixed amount, we could move it over by the length of the longest track name. This is sort of like how we already lay out the “arrow” (->) in the metadata change display. This could make things more compact—although I’m not sure that’s better or worse than what you already have. I’d be interested in your thoughts, though!

@jams2

This comment has been minimized.

Copy link
Contributor Author

commented Jun 19, 2018

Yep, that might be a better option. I had a soft-limit of 80 characters in the back of my mind, but that's pretty arbitrary. I'll look into it.

@jams2

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2018

beet
@sampsyo made your suggested change, looks good I think!

@sampsyo

This comment has been minimized.

Copy link
Member

commented Jun 20, 2018

Nice!! That looks fantastic. Great work; and thanks for the insight that this could be made better!

@sampsyo sampsyo merged commit a62e4ef into beetbox:master Jun 20, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jams2

This comment has been minimized.

Copy link
Contributor Author

commented Jun 20, 2018

Happy to help out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.