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

GUI fixes: Elide playback name label if needed and… #163

Merged
merged 1 commit into from
Aug 19, 2020

Conversation

tsujan
Copy link
Member

@tsujan tsujan commented Aug 16, 2020

If the name is too long, elide it centrally, so that a horizontal scrollbar isn't needed (and isn't shown).

Also, don't force a height on progressbars because some widget styles may not show any bar with a forced height.

Closes #99 and closes #75

If the name is too long, elide it centrally, so that a horizontal scrollbar isn't needed (and isn't shown).

Also, don't force a height on progressbars because some widget styles may not show any bar with a forced height.

Closes #99 and closes #75
@tsujan tsujan requested a review from yan12125 August 16, 2020 18:57
@tsujan
Copy link
Member Author

tsujan commented Aug 16, 2020

Before:

before

After:

after

@yan12125
Copy link
Member

The class ElidedLabel seems the same as the one from libfm-qt [1]? I wonder if both classes can be merged into one.

[1] https://github.com/lxqt/libfm-qt/blob/master/src/fileoperationdialog_p.h

@tsujan
Copy link
Member Author

tsujan commented Aug 17, 2020

The class ElidedLabel seems the same as the one from libfm-qt?

Yes. I'd made it for libfm-qt's file operation dialog and for a similar reason. Here, I just copy-pasted it.

I wonder if both classes can be merged into one.

pavucontrol-qt and libfm-qt have no common LXQt dependency (except for lxqt-build-tools). IMHO, adding a dependency just for a simple code like this would be too much.

I think, at some point, Qt itself should add this property to QLabel (but, apparently, there's no plan for it). It reminds me of QLineEdit's clear button, which was added to Qt5, at last.

@yan12125
Copy link
Member

Copying this small class for two times is fine, but I guess such a class will appear in more LXQt projects, and merging will be better then.

For now I'm fine with the current approach. Feel free to merge if you wish.


I think, at some point, Qt itself should add this property to QLabel (but, apparently, there's no plan for it)

Looking into Qt's bug tracker, and I feel this comment implies that will be a complex feature

if the label was set to use rich text (html: clickable URLs, coloring, etc.), the rich text support shouldn't suddenly disappear when eliding kicks in, it should remain rich text support while eliding.

(https://bugreports.qt.io/browse/QTBUG-1277#comment-221318)

@tsujan
Copy link
Member Author

tsujan commented Aug 19, 2020

Looking into Qt's bug tracker, and I feel this comment implies that will be a complex feature

Yes, because of the rich text, which is the whole point. With a plain text, a simple subclassing (like the above) does the job.

For now I'm fine with the current approach. Feel free to merge if you wish.

Thanks!

@tsujan tsujan merged commit c900346 into master Aug 19, 2020
@yan12125 yan12125 deleted the eliding_name_label branch August 19, 2020 16:43
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.

Eliding is needed sometimes No volume meter with some Qt styles.
2 participants