Tracks: use clicked column to focus property editor in Track Info dialog #14036
Conversation
ronso0
force-pushed
the
trackmenu-trackinfo-focus
branch
from
95d3905 to
7024184
Compare
src/widget/wtrackmenu.cpp
Outdated
| @@ -2567,6 +2569,7 @@ void WTrackMenu::slotShowDlgTrackInfo() { | |||
| [this]() { | |||
| if (m_pDlgTrackInfo.get() == sender()) { | |||
| m_pDlgTrackInfo.release()->deleteLater(); | |||
| setTrackPropertyName(); | |||
There was a problem hiding this comment.
This one was not obvious for me. How about clearTrackPropertyName();?
It is a bit of an odd logic that the user of m_trackProperty needs to clean up. Did you consider to pass the property along with loadTrack()? This way every caller has to descide about the property to focus.
I have not checked how big this change would be, so it is just a suggestion.
There was a problem hiding this comment.
I just didn't want to introduce yet another method.
We could also simply reset it directly:
m_trackproperty = QString();
There was a problem hiding this comment.
Do you think the cleanup is an issue if it's done close to where we call setFocus()?
Did you consider to pass the property along with loadTrack()?
Then we'd need to override QDialog::show() and call setFocus() there.
There was a problem hiding this comment.
We could also not reset it, so it'll be reused in case no property is specified?
Might be unexpected if it was "bpm" before (BPM tab would be selected).
There was a problem hiding this comment.
m_trackproperty = QString(); is just fine for a quick merge. If my other idea is not a short thing, just drop it.
ronso0
force-pushed
the
trackmenu-trackinfo-focus
branch
from
7024184 to
4a22d6d
Compare
follow-up for #13841
and a small tweak for DlgTrackInfo (single track)