Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Sidebar overhaul #454

Merged
merged 28 commits into from
Dec 6, 2024
Merged

Sidebar overhaul #454

merged 28 commits into from
Dec 6, 2024

Conversation

bkloeckl
Copy link
Contributor

Change sidebar UIWidget from List to Table in order to display unique indices of datasets and add a close-button in each row.

@cbrnr
Copy link
Owner

cbrnr commented Dec 4, 2024

I have two suggestions regarding the behavior of this implementation:

  1. The "x" button should disappear when the mouse is not hovering over any data set. Currently, the "x" stays where it last was when the mouse leaves the widget, which I find distracting.
  2. Closing a dataset is not yet reflected in the history. I know that this was also the case before this PR, but maybe you could add a history entry for the close data method as part of this PR?

I haven't looked in detail at the changes (will do that soon), but because you changed model.remove_data(), I was wondering if things like self.model.index -= 1 after self.model.remove_data() still work as intended (see here)? There's even a line that is commented out (see here). In total, there are three occurrences of this pattern (in interpolate_bads(), epoch_data(), and change_reference()). Can you check what's up with those?

Also, as we've already discussed, I think you do not have to duplicate the drag and drop behavior (for dropping files), since this is already implemented in MainWindow.

I'll review the changes in detail after we've addressed these points.

@bkloeckl
Copy link
Contributor Author

bkloeckl commented Dec 4, 2024

I haven't looked in detail at the changes (will do that soon), but because you changed model.remove_data(), I was wondering if things like self.model.index -= 1 after self.model.remove_data() still work as intended (see here)? There's even a line that is commented out (see here). In total, there are three occurrences of this pattern (in interpolate_bads(), epoch_data(), and change_reference()). Can you check what's up with those?

The changes in the remove_data() function now allow to pass an index parameter, necessary for deleting rows, not currently selected. Its functionality, when no index is passed, remains however unchanged.

@cbrnr cbrnr linked an issue Dec 4, 2024 that may be closed by this pull request
@cbrnr cbrnr merged commit af11dc5 into cbrnr:main Dec 6, 2024
6 checks passed
@cbrnr
Copy link
Owner

cbrnr commented Dec 6, 2024

This is a very nice change, I like it a lot! Thanks @bkloeckl!

@bkloeckl bkloeckl deleted the sidebar branch December 6, 2024 12:07
cbrnr pushed a commit that referenced this pull request Feb 3, 2025
* Change the append dialog appearance, get_compatibles and append_data functions to include original indices used for identifying the data

* Add changelog entry

* Ruff formating

* custom TableWidget class for sidebar

* drag&drop and tests

* core functionalities done

* add close-button to sidebar table

* styling and changelog

* disable hover/mouse-tracking

* Revert "Change the append dialog appearance, get_compatibles and append_data functions to include original indices used for identifying the data"

This reverts commit 6a9917e.

* Revert "Add changelog entry"

This reverts commit 916f803.

* Revert "Ruff formating"

This reverts commit 4e54cd8.

* append dialog revert

* small close-icon on hover

* style and delete old icon

* resolve merge conflicts

* small changes and clean up code, changed sidebar index styling, remove vertical header

* added load file by drag&drop functionality

* tiny fix: missing styling in append-dialog

* x button dissapears when mouse does not hover over any row anymore

* add remove_data() history entry

* delegate drag&drop file functionality to parent class to remove duplicated code

* ruff error: removed unused import

* Set SingleSelection mode

* Move to widgets subpackage

* Fix changelog entries

* Sidebar index is not editable

* Remove empty line
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add "close" button for data sets in sidebar
2 participants