Skip to content

Add Number of tracks to Tracks label in Library#14552

Merged
ronso0 merged 1 commit into
mixxxdj:mainfrom
NeuroXS:feature-track-count
Sep 14, 2025
Merged

Add Number of tracks to Tracks label in Library#14552
ronso0 merged 1 commit into
mixxxdj:mainfrom
NeuroXS:feature-track-count

Conversation

@NeuroXS
Copy link
Copy Markdown
Contributor

@NeuroXS NeuroXS commented Mar 26, 2025

@JoergAtGithub
Copy link
Copy Markdown
Member

Welcome at Mixxx!
As a first-time contributor we need you to sign the Mixxx Contributor Agreement and comment here when you have done so. It gives us permission to distribute your contribution under the GPL v2 or later license and the Apple Mac App Store. It is also helpful for us to have contact information for contributors in case we may need it in the future.

@NeuroXS
Copy link
Copy Markdown
Contributor Author

NeuroXS commented Mar 26, 2025

Hey, I've signed the agreement :)

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Mar 26, 2025

This doesn't build if ENGINEPRIME is off.

You need to remove the #ifdef ENGINEPRIME around MixxxLibraryFeature::bindSidebarWidget(), move QPointer<WLibrarySidebar> m_pSidebarWidget; declaration etc.

Then it works.

Comment thread src/library/mixxxlibraryfeature.cpp Outdated
@github-actions
Copy link
Copy Markdown

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions Bot added the stale Stale issues that haven't been updated for a long time. label Jun 26, 2025
@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Jun 26, 2025

While this is just a bandaid for a status bar #10382, it does job for Tracks (and works as expected).
What do you think @mixxxdj/developers ?

LGTM btw.
@NeuroXS Do you think you'll find time to finish this?
If yes, please squash the commits and rebase onto main (and resolve the conflicts).

@github-actions github-actions Bot removed the stale Stale issues that haven't been updated for a long time. label Jun 27, 2025
Comment thread src/library/mixxxlibraryfeature.cpp Outdated
@NeuroXS
Copy link
Copy Markdown
Contributor Author

NeuroXS commented Sep 11, 2025

Hi, I have free time if you think there's something I can do related to this PR, would be cool to have my first merge in Mixxx :D

Last thing i did was solve the conflicts and do the change requested
Sorry, I'm new to github and this is my first PR, let me know if I'm doing a mistake :)

Copy link
Copy Markdown
Member

@ywwg ywwg left a comment

Choose a reason for hiding this comment

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

looks good, congrats on your first contribution!

Copy link
Copy Markdown
Member

@ronso0 ronso0 left a comment

Choose a reason for hiding this comment

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

Thanks. I took another look and like to improve this a bit.
(the repaint nit is just a question)

Comment thread src/library/mixxxlibraryfeature.cpp Outdated
if (m_pSidebarWidget) {
m_pSidebarWidget->repaint();
}
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

repaint might be cheap, but is there another way to trigger repaint of just the Tracks item?

@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 11, 2025

Oh, and please rebase to give the actual track count commit a meaningful message ("Final commit" is .. not that helpful if you try to guess the change when looking at a git log ; )
-- drop the merge commit while doing so

@NeuroXS NeuroXS force-pushed the feature-track-count branch 5 times, most recently from 9a0599b to 0510c0e Compare September 12, 2025 20:23
@NeuroXS NeuroXS requested a review from ronso0 September 13, 2025 14:07
Comment thread src/library/mixxxlibraryfeature.cpp Outdated
Comment on lines +198 to +203
if (m_pTracksTreeItem && m_pSidebarModel) {
QModelIndex rootIndex = m_pSidebarModel->getRootIndex();
QModelIndex tracksIndex = m_pSidebarModel->index(
m_pTracksTreeItem->parentRow(), 0, rootIndex);
m_pSidebarModel->triggerRepaint(tracksIndex);
}
Copy link
Copy Markdown
Member

@ronso0 ronso0 Sep 13, 2025

Choose a reason for hiding this comment

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

Thanks for the update, and sorry for kind of turning in circles here.
Turns out my performance concern was not relevant in the first place because Qt is smart and only repaints 'dirty' regions, no matter what we request to repaint.

TL;DR
The cleanest update request we can issue is
emit featureIsLoading(this, false /* don't re-select */);
This signal meant to update the sidebar title for Traktor, iTunes and other external features that do dynamic loading.

NeuroXS#1


Btw your current implementation won't work as intended.
TreeItem->parentRow() will return -1 for root items, so tracksIndex is invalid and thereby we'd still request to repaint the entire sidebar.

@NeuroXS NeuroXS force-pushed the feature-track-count branch from a7a2e95 to 81741b7 Compare September 14, 2025 03:54
@NeuroXS NeuroXS requested a review from ronso0 September 14, 2025 04:08
@ronso0
Copy link
Copy Markdown
Member

ronso0 commented Sep 14, 2025

LGTM, thanks!

@ronso0 ronso0 merged commit d695cc6 into mixxxdj:main Sep 14, 2025
14 checks passed
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.

4 participants