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

Use unique indices to identify datasets for appending #449

Merged
merged 29 commits into from
Dec 3, 2024

Conversation

bkloeckl
Copy link
Contributor

Change the append dialog, get_compatibles and append_data functions to include original and unique indices used for identifying the data.

@cbrnr
Copy link
Owner

cbrnr commented Nov 18, 2024

Thanks @bkloeckl! The current changes lack the ability to sort datasets (via drag and drop) in the Source and Destination lists though, which I think we should not remove.

I would also keep it simpler, instead of using a table, I'd stick to a list, but just add the index to the label. This could also be done for the sidebar then. I'm not sure how and if such a label can be formatted nicely, e.g. it would be nice to have

  • numbers right-aligned
  • numbers use some lighter color (not black or white)

Something like this:

Screenshot 2024-11-18 at 09 17 41

If this requires a table (which might very well be the case), I'm sure that the style can be adapted so that it doesn't contain a header row or any borders.

@cbrnr
Copy link
Owner

cbrnr commented Nov 29, 2024

I found an issue with the current implementation:

Screen.Recording.2024-11-29.at.09.13.36.mov

@cbrnr
Copy link
Owner

cbrnr commented Nov 29, 2024

Another thing that could be improved is that currently, dragging one or more items from the source table to the destination table (or vice versa) shows the drop hint (the black line where the item would be inserted), but unfortunately dropping to the other table does not work. I think it would be very nice if that worked (then users could operate the dialog only via drag and drop and would not even need the arrow button, which is still a useful backup/alternative).

@cbrnr
Copy link
Owner

cbrnr commented Nov 29, 2024

Finally, I was wondering if you really have to implement the drag and drop on that level, or if you could maybe just connect the rowsMoved signal to simplify (as is done here for drag and drop in the sidebar)?

@bkloeckl
Copy link
Contributor Author

Finally, I was wondering if you really have to implement the drag and drop on that level, or if you could maybe just connect the rowsMoved signal to simplify (as is done here for drag and drop in the sidebar)?

For the sidebar, the data and its order are managed by the model and then visually updated. The append-dialog has to handle the reordering on its own. This implementation is also the base for the latest update, where drag&drop works across the source and destination tables.

@cbrnr cbrnr merged commit 807c8e4 into cbrnr:main Dec 3, 2024
6 checks passed
@cbrnr
Copy link
Owner

cbrnr commented Dec 3, 2024

Thanks @bkloeckl!

@bkloeckl bkloeckl deleted the duplicate_names branch December 6, 2024 12:08
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

* ruff formating

* ruff formating imports

* adapted tests

* adjust indices when needed after auto_duplicate()

* adjust test

* delete wrong test_model, commit correct one

* change append dialog styling and add drag-drop functionality

* table styling changes

* align correction

* Fix changelog

* Improve readability

* Minor fix

* Fix style

* fix move-bug by sorting rows

* drag&drop between source and destiantion table now possible

* Style

* ruff error?

* Fix style

* Reduce redundancy

* Use global constant for row height

* Remove viewport update (not necessary)

* Remove from paintEvent

* Simplify (no set necessary)

* Restore horizontal line

* Remove leftover code

* Remove another set
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants