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

Tracks view: keep current item visible when the view shrinks vertically #11273

Merged
merged 2 commits into from Jan 24, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Feb 13, 2023

Just a small helper to fix an issue I had on my list for a long time:
if you toggle/expand a hidden/collapsed container (waveforms, effects, sampler) the track view shrinks which can push the track selection out of sight.
This commit makes sure it is pulled into view again if required.

Testing

Pick a track at the bottom of the view, and make sure there are some hidden skin regions to expand.
That track should reman visible both when showing a hidden region, as well as when manually (slowly) dragging the waveform splitter downwards.

Note: even though this handles the currentIndex (focused item, not the selection) this doesn't matter unless you use Ctrl + keys to move independent from the selection.

@github-actions github-actions bot added the ui label Feb 13, 2023
@ronso0 ronso0 added the library label Feb 13, 2023
@ronso0 ronso0 force-pushed the tracks-keep-selected-item-in-view branch from e48b754 to 34c3079 Compare February 14, 2023 21:22
@JoergAtGithub

This comment was marked as off-topic.

@ronso0

This comment was marked as off-topic.

@ronso0 ronso0 force-pushed the tracks-keep-selected-item-in-view branch from 34c3079 to 038cbb0 Compare March 19, 2023 21:09
@JoergAtGithub
Copy link
Member

Code looks is much better understandable now!
I did a short test on Windows11, and it behaves as expected!

@daschuer
Copy link
Member

daschuer commented Mar 20, 2023

The pre-commit failure is unrelated.

Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

Thank you for this nice addition. I have added some minor comments for improvement.

}

QModelIndex currIndex = currentIndex();
int rHeight = rowHeight(0);
Copy link
Member

Choose a reason for hiding this comment

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

This can be moved closer to usage.
I think it can become
'rowHeight(currRow)`
to make it work even if we have different row heights one day.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's used right below.
I looked at the Qt sources and noticed !currIndex.isValid() can be replaced with currRow < 0 || rowHeight(currRow()) = 0

// y-pos of the top edge, negative value means above viewport boundary
int posInView = rowViewportPosition(currRow);
// Check if the row is visible.
// Note: don't use viewport()->height() because that may already have changed (??)
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove (??)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

return;
}

// TODO(xxx) When resizing, the top row is static, at least on Linux.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is correct and the TODO can be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

okido.

@ronso0 ronso0 added this to the 2.4.0 milestone May 8, 2023
@github-actions
Copy link

github-actions bot commented Aug 7, 2023

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 Aug 7, 2023
@JoergAtGithub JoergAtGithub removed the stale Stale issues that haven't been updated for a long time. label Aug 9, 2023
Copy link

github-actions bot commented Nov 8, 2023

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 Nov 8, 2023
@ronso0 ronso0 removed this from the 2.4.0 milestone Dec 3, 2023
@github-actions github-actions bot removed the stale Stale issues that haven't been updated for a long time. label Dec 4, 2023
@daschuer
Copy link
Member

This still works good. Thank you.

@daschuer daschuer merged commit c17c616 into mixxxdj:2.4 Jan 24, 2024
11 of 12 checks passed
@ronso0 ronso0 deleted the tracks-keep-selected-item-in-view branch January 25, 2024 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants