WIP: Canvas Scroll, Pan & Other Item handling improvements

[giacomoalbe]

Summary / How this PR fixes the problem?

With this PR I'd like to implement some Canvas actions and movements and to improve the usability of the Canvas tools (move, resize, rotate, etc) as stated in #212, #217 and #206.

Here's the list of desidered/already done implementations:

• Reordered methods in Canvas to be more consistent (item add methods, event handling methods, ecc)
• Pan Canvas with Space + mouse move
• Drag item everywhere on the canvas
• Add rotation step increments while holding CTRL

I know @Alecaddd is working on this too, but those particular actions were missing from his PR, so I thought about implementing them.

If you think with PR is too broad, I can simply split in two or even more, or if you think there's should be more urgent work to do on other topics, please let me know and I'll be working on that instead.

[albfan]

I discourage to mix whitespace corrections and method reorder when adding functionality. There's lot of movement this days, and that just make merges hard.

You can always fix those things but please do in separate commits, so we can see what you're really adding or fixing

[giacomoalbe]

Hi @albfan, thanks for the response! I've seeen that there's a lot of movement these days, I'm very happy about it.

I can get your point, but I think that my little reorder is simple enough not to break things. I already merged it in my repo from current master and it has not been that hard to fix the conflicts.

Nevertheless, I'll be marking each edit accordingly and I'll put those changes in different commits.

[albfan]

Yes is just a general advice. surely we can deal with this changes. Thanks for working on this!

[Alecaddd]

Thanks for working on this, I'll review it later and see how it behaves.
As you noticed, I'm working on something similar in canvas actions, more focused on the canvas rather than the objects, so I think some merging issues are likely to happen.

[giacomoalbe]

Hi @Alecaddd, thanks for your answer, as well.

I implemented the space pan behaviour and the constrained rotation with CTRL, but now I think it's wiser to stop here and maybe open another PR for the other things (like centered resize and aspect ratio contrained resize, since those actions haven't been discussed yet, too), in order to avoid unneeded conflicts during merge.

I expect the zoom with CTRL + wheel and pan with middle mouse to be implemented in your PR, so I just don't want to make any "double" in here. Am I right?

I would like to work on something else for the 1.0 Milestone, maybe the Artwork/multiple select topic might be more suitable in order to minimize conflicts?

[Alecaddd]

Indeed, that's exactly what I was going to suggest.
Better keep small PRs, and even they end up being big, it's always better to keep the focus on 1 section/behaviour, as broad PRs are hard to review and back-up.

I have some edits to suggest, so stay tuned :D

[Alecaddd]

Just a few small changes

[Alecaddd]

A couple of more edits, please.

• Also, when the Spacebar is pressed and hold, the hover effect on an object shouldn't happen as it gives a false feedback to the user. Add that exception.
• Another thing is that we should keep our code consistent in the way we do things. Whenever there's a canvas action that changes the default behaviour, we update the enum EditMode. Instead here you're using the pan_mode_enabled, which breaks the consistency. Let's update that to follow our approach.

[aberba]

What is happening?

[Alecaddd]

Hey @giacomoalbe, sorry for the delay in reviewing this.
I pushed a rebased version from master on this branch: https://github.com/akiraux/Akira/tree/giacomoalbe-canvas-scroll-and-pan

Upon testing it the scrolling and panning don't work as expected.
Would you be able to take a look at it and rebase your PR?
Thank you so much.

[Alecaddd]

I guess this can be closed, right @giacomoalbe?