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

cut/copy/paste actions #140

Closed
kbengs opened this issue Feb 9, 2020 · 16 comments
Closed

cut/copy/paste actions #140

kbengs opened this issue Feb 9, 2020 · 16 comments
Labels
feature Request for a new feature
Milestone

Comments

@kbengs
Copy link
Member

kbengs commented Feb 9, 2020

Would be nice to have option to cut / paste selected pages when right clicking. It is now a bit hard to move for example the last page to the beginning when there is a lot of pages..

@jeromerobert
Copy link
Member

jeromerobert commented Feb 9, 2020

Yep, this was on my (hidden) to do list.

The problem is not that we don't have cut/copy/paste actions in the menu, the problem is that we don't have them at all. The current drag & drop must be reimplemented to separate the drag & drop behavior from the cut/copy/paste code, so it could be reused in menus (popup and menu bars) and have ctrl+cxv shortcuts.

It would also make #67 easier to implement.

@jeromerobert jeromerobert changed the title Idea: Cut + paste option in right click menu? cut/copy/past actions Feb 9, 2020
@jeromerobert jeromerobert added the feature Request for a new feature label Feb 9, 2020
@dreua
Copy link
Member

dreua commented Feb 10, 2020

Before thinking about code I see a UX problem here: We currently do not have a cursor between two pages but only a selection of pages. Think of text editing: The cursor is usually not on but between two characters and you can move it in front of the first and after the last character. We could decide to make pasted pages appear before or after the current selection but either way it wouldn't be possible to paste as the first and the last page.

@jeromerobert
Copy link
Member

As I'm not that comfortable with UX I was thinking to something like:

  • A past action (either before or after but not both) which would be bound to ctrl+v
  • A special past action which would manage before, after, and interleave (for Merge double sided scanned document #67)

I don't know yet if special past should popup a dialog or be split into different actions (into a submenu).

@kbengs
Copy link
Member Author

kbengs commented Feb 15, 2020

One way of doing it using partly already available functions (I suppose) would be first to do a modified version of the "export selection" that would save selected pages to temporary location (without having to supply a place and file location). Then delete selected pages. Then you would select the page where you would like to paste. Then choose "paste", and it would paste for example after selected page.

Another thing a bit related to this, I would suggest in pdfarranger.py on line 853 "autoscroll_area = 40" would be increased to 80.

@kbengs
Copy link
Member Author

kbengs commented Feb 24, 2020

I have been working on this. If someone wants to see it is in kbengs/pdfarranger/kben/raw fork/branch. It paste after selected item(s) (after last selected item). Paste to other instance does not work. But otherwise it seems to work. Should I continue or stop? :)

dreua added a commit to dreua/pdfarranger that referenced this issue Feb 24, 2020
Prevent GObject.source_remove being called with an invalid id when
the timer has already been stopped. This can be triggered
by adding another PDF via drag and drop or by testing kbengs' work
on pdfarranger#140.

Warning message:
```
pdfarranger/pdfarranger.py:491: Warning: Source ID 26 was not found when attempting to remove it
  GObject.source_remove(self.progress_bar_timeout_id)
```
@dreua
Copy link
Member

dreua commented Feb 24, 2020

I just gave it a try and it definitely feels like a good addition! 👍 Like I mentioned before we now have the problem, that you can't paste something before the first page. I wonder if we could add a selectable dummy before the first page that could be selected by arrow buttons or clicking so that pages pasted there would end up at the beginning. (I may be aiming high here, a "paste before" action would be probably easier to implement and sufficiently usable when mapped to Ctrl+Shift+V.)

For allowing copy and paste between instances you could serialize all the data to a string and put that in the actual clipboard, similar to the current implementation of drag and drop. Like Jerome already mentioned, the current drag and drop code should be reused in that case. (And it probably needs some refactoring beforehand.)

@kbengs
Copy link
Member Author

kbengs commented Feb 27, 2020

Ok, thanks for comments. I have done more changes now:
-added "Paste special" menu item. It includes "Paste before" item. (so that we maybe could add "paste interleaved" to it later)
-added ctrl + shift + v for paste before
-added cut, copy, paste and clear functions that also drag and drop uses. "dnd intern" is unchanged, as it works very differently.
-paste to other instance possible, also if it it empty
-drag to empty instance also possible
-clipboard uses Gtk.Clipboard

Dont know of any problem with it. Not tested anything on windows.

jeromerobert pushed a commit that referenced this issue Feb 28, 2020
Prevent GObject.source_remove being called with an invalid id when
the timer has already been stopped. This can be triggered
by adding another PDF via drag and drop or by testing kbengs' work
on #140.

Warning message:
```
pdfarranger/pdfarranger.py:491: Warning: Source ID 26 was not found when attempting to remove it
  GObject.source_remove(self.progress_bar_timeout_id)
```
@jeromerobert
Copy link
Member

jeromerobert commented Feb 29, 2020

  • There are some noisy/useless changes in pdfarranger.ui which should be removed (probably a Glade side effect)
  • self.cb is too short for a such large scope variable. self.clipboard would be better.
  • May be I'm wrong but I think that the on_action_copy function should not set_unsaved nor update the undo manager as it does not modify the data.
  • Refactoring/factorization with drag/drop is still to be made but there are already a lot of changes here, so it could be done in a second time (and by someone else). But if you want to make it in the same PR (which is also fine), please keep in separate commits.
  • Try to reorganize your branch before raising a PR. It should reflect the changes you provide, not the errors and trials you made to achieve this changes. Currently I have to squash all your commit to a single one to be able to read what you want to contribute. It will also help you to find better commit messages. git rebase -i, git gui, git crecord are some of the commands you may find useful to reorganize your branch. https://github.com/mockito/mockito/wiki/Using-git-to-prepare-your-PR-to-have-a-clean-history may also help. The more changes you make before you reorganize, the harder the reorganization will be.
  • I don't plan to merge this before 1.4.2 as it's too disruptive for bug fix release. It will be for 1.5.0.

@kbengs
Copy link
Member Author

kbengs commented Mar 6, 2020

Thanks for comments. I have cleaned the history and fixed the things you commented. It is now in kben/cutcopypaste branch. If/when I feel I have no more to add to this, is it too early to make a PR, too far from 1.5?
Something still needs to be done to language files.

kbengs added a commit to kbengs/pdfarranger that referenced this issue Mar 7, 2020
@jeromerobert
Copy link
Member

Yes, it looks perfect ! I will probably release 1.4.2 tomorrow so you can make your PR.

kbengs added a commit to kbengs/pdfarranger that referenced this issue Mar 9, 2020
kbengs added a commit to kbengs/pdfarranger that referenced this issue Mar 11, 2020
kbengs added a commit to kbengs/pdfarranger that referenced this issue Mar 12, 2020
@kbengs
Copy link
Member Author

kbengs commented Mar 20, 2020

I couldn't resist the paste interleave..,
this is how it works:

  • user copies or cuts the pages he wants to add interleaved (to clipboard)
  • when pasting, if nothing is selected and user choose:
    • paste interleave odd, pages are added from beginning, first, third,...
    • paste interleave even, pages are added from beginning, second, fourth,...
  • when pasting, if something is selected and user choose:
    • paste interleave odd, first page is added before first selected page, second page is added after first selected page,...
    • paste interleave even, first page is added after first selected page, ...
    • how many pages are selected when pasting does not care, pasting will continue beyond last selected page (if needed)
  • if pages in iconview and pasted pages are not equal number, remaining pages will be added at end/left at end

What do you think about this behavior?

All is in same branch.

@dreua
Copy link
Member

dreua commented Mar 20, 2020

I'll give it a try and see how it feels asap

@kbengs
Copy link
Member Author

kbengs commented Mar 21, 2020

I also changed paste after/before so that:

  • When pasting after/before:
    • if nothing is selected, pages are added to end/to beginning
    • if something is selected, pages are added relative to selection
  • removed (selection) from paste menutext

@jeromerobert
Copy link
Member

@kbengs It looks good. Do you have any objection to me merging it now?

@kbengs
Copy link
Member Author

kbengs commented Mar 22, 2020

No I am happy with it, you can merge it. 👍

@jeromerobert jeromerobert added this to the 1.5 milestone Mar 22, 2020
@dreua
Copy link
Member

dreua commented Mar 23, 2020

That's great, good work @kbengs 👍 PDF Arranger is getting really fancy now with drag and drop, undo, redo and now copy & paste 😀
Also the paste odd/even function works really good for me!

@jeromerobert jeromerobert changed the title cut/copy/past actions cut/copy/paste actions Mar 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Request for a new feature
Projects
None yet
Development

No branches or pull requests

3 participants