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

fix deletion of effect chain presets created in current session #11886

Merged
merged 6 commits into from
Oct 16, 2023

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Aug 29, 2023

  • fix occasional issue in Preferences -> Effects:
    clicking any of the preset action buttons (Rename, Delete, ..) does sometimes clear the selection in the list views above, i.e. the buttons won't do anything.
    Looks like the events are processed in unpredictable order: sometimes the Delete button gets focus and the list focus as well as the selection is cleared, sometimes the selection is cleared after the Delete dialog is closed (aborted, so the selected item is still in the list and selection should not change

@github-actions github-actions bot added the ui label Aug 29, 2023
@ronso0 ronso0 added this to the 2.4.0 milestone Aug 29, 2023
@ronso0
Copy link
Member Author

ronso0 commented Aug 29, 2023

This can be merged as is, though I need to figure what's wrong with the focus behaviour / event processing in that dialog.

Can anyone else reproduce this (I'm using Qt 5.12.8 on Ubuntu 20.04):

  • open Pref -> Effects
  • select one chain preset in the Quick Effect Chain Presets list
  • click Delete
  • list selection is cleared, Delete button does nothing

@ronso0 ronso0 linked an issue Aug 29, 2023 that may be closed by this pull request
@github-actions github-actions bot added the ui label Aug 29, 2023
@ronso0
Copy link
Member Author

ronso0 commented Aug 29, 2023

Done, ready for review.

clicking any of the preset action buttons (Rename, Delete, ..) does sometimes clear the selection in the list views above, i.e. the buttons won't do anything.
Looks like the events are processed in unpredictable order: sometimes the Delete button gets focus and the list focus as well as the selection is cleared, sometimes the selection is cleared after the Delete dialog is closed (aborted, so the selected item is still in the list and selection should not change

I changed the eventFilter return value to QWidget::eventFilter and can't reproduce the issue anymore.
Furthermore, the lists are now reloaded only when the state actually changed, i.e. when presets are renamed, deleted or imported.

@ronso0 ronso0 changed the title fix deletion of just effect chain presets created in current session fix deletion of effect chain presets created in current session Aug 29, 2023
m_pFocusedChainList->selectionModel()->selectedRows(0).count() > 1) {
auto* pSelModel = m_pFocusedChainList->selectionModel();
auto selIndices = pSelModel->selectedIndexes();
if (!pSelModel->hasSelection() || selIndices.count() > 1) {
Copy link
Member

Choose a reason for hiding this comment

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

why ||?

Copy link
Member Author

@ronso0 ronso0 Aug 30, 2023

Choose a reason for hiding this comment

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

This should clear the info labels and return if (selIndices.count() != 1).

I pushed the simplified version. Thanks!

@Holzhaus
Copy link
Member

There are merge conflicts now.

@ronso0
Copy link
Member Author

ronso0 commented Sep 30, 2023

Conflicts resolved.

In case the new tr string is an issue I can remove 46f2f91 and open a separate PR against main.

@ronso0
Copy link
Member Author

ronso0 commented Oct 15, 2023

pa-ping..

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.

LGTM and still works! Thank you.

@daschuer daschuer merged commit c727ccd into mixxxdj:2.4 Oct 16, 2023
13 checks passed
@ronso0 ronso0 deleted the effect-chain-preset-delete branch October 16, 2023 21:05
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.

can not delete custom effect chain
4 participants