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

[macOS] Canvas/Timeline shortcut issues #324

Open
pgilfernandez opened this issue Nov 14, 2024 · 29 comments
Open

[macOS] Canvas/Timeline shortcut issues #324

pgilfernandez opened this issue Nov 14, 2024 · 29 comments
Labels
bug Something isn't working mac
Milestone

Comments

@pgilfernandez
Copy link

In some scenarios you want to duplicate keyframes and you end up duplicating layers/groups/paths as well.

For instance, if you select a square in the viewport and then you go to the keyframes timeline, select a keyframe, scrub the "time line" and duplicate (by menu or shortcut) the result is that you are duplicating both the path and creating a duplicate of the keyframe...

Following this example, if you select a square in the viewport (it gets selected in the timeline) but you first click in an empty space then you are kind of "unselecting all". Now you go back to the keyframes timeline, select a keyframe, scrub the "time line" and duplicate (by menu or shortcut) the result is now is correct, just the keyframe gets duplicated.

The proposed behavior here would be that if your mouse is in the timeline, if you run the "duplicate" command, if there is a keyframe selected the only thing that gets duplicated is the keyframe and not any other thing but keyframes. In case there is no keyframe selected then it could duplicate layers/groups/paths...

Could you reproduce this?

Thanks!

@pgilfernandez
Copy link
Author

I got time to record a video:

duplicacting.mp4

@KirbysDarkNebula
Copy link

I'm unable to reproduce this issue in 1.0.0-beta1, the only problem I found was that keyframe selection would not get removed after clicking on the viewport which leads to confusing behaviour

Doing the same, getting no object + keyframe duplication, only keyframe duplication:
https://github.com/user-attachments/assets/fb5e5c23-14cb-4513-aa9d-300a702a8c4e

@pgilfernandez
Copy link
Author

Then it only happens in macOS... @KirbysDarkNebula, what OS are you using?

@KirbysDarkNebula
Copy link

I'm using Linux with the Appimage release

@pgilfernandez
Copy link
Author

I'm using Linux with the Appimage release

Thanks!
OK, then it happens in macOS and I guess it will not happen on Windows neather...

@rodlie
Copy link
Member

rodlie commented Nov 18, 2024

Seems like another awesome macOS issue 😄

the only problem I found was that keyframe selection would not get removed after clicking on the viewport which leads to confusing behaviour

Keyframe selection has nothing to do with the viewport.

@rodlie rodlie added the mac label Nov 18, 2024
@pgilfernandez
Copy link
Author

@rodlie could you point me out where to look at in order to try fixing this for macOS? I would like to try...

Thanks

@rodlie
Copy link
Member

rodlie commented Nov 21, 2024

I have not looked at the issue yet, currently busy with work stuff.

I assume that the copy keyframe action does not reserve/block the key event and you end up with multiple actions on mac.

Will be more involved with the project this weekend (as I want to push for a last beta release next week).

@pgilfernandez
Copy link
Author

Don't worry, I'll investigate. I wanted it just to get faster to the "target" and find a fix.

Nice to hear about a second beta. I hope you have time before to have a look at my PRs and I hope you introduce the experimental macOS build 🙏🏻😄 hehehe

@rodlie
Copy link
Member

rodlie commented Nov 28, 2024

Been looking at this issue and it will probably not be a simple fix.

The custom class NoShortcutAction still register shortcuts on Mac, so that's why keys and objects are copied when doing cmd+c on the timeline (since cmd+c is set as shortcut on a NoShortcutAction :smile )

This is fixable, all NoShortcutAction's will need to be reviewed and verified that they works as they should on Mac, and what happens when we disable all shortcuts on them.

But, we have one more issue, duplicated copy.

When pressing cmd+c in the timeline you will notice that focus switches to the window, and that triggers a new copy action. For some reason focus change when pressing cmd+c on Mac, this is bad as Friction is focus based for shortcuts etc.

@rodlie rodlie changed the title Layers/groups/paths duplicate when you are duplicating keyframes [macOS] objects/keys CMD+C issues Nov 28, 2024
@rodlie rodlie added the bug Something isn't working label Nov 28, 2024
@pgilfernandez
Copy link
Author

I understand, not easy fix.

I was trying to filter the clipboard so that just paste the type of element I wanted (keyframes for instance if the timeline was on focus) but if you say the focus changes when the key shortcut is triggered then it would never work...

rodlie added a commit that referenced this issue Nov 30, 2024
@rodlie rodlie added this to the 1.0.0 milestone Nov 30, 2024
@rodlie
Copy link
Member

rodlie commented Nov 30, 2024

Fixed in fdb4455

Note that there might be more broken shortcuts on macOS.

@rodlie rodlie closed this as completed Nov 30, 2024
@rodlie rodlie changed the title [macOS] objects/keys CMD+C issues [macOS] Canvas/Timeline shortcut issues Dec 3, 2024
@rodlie rodlie reopened this Dec 3, 2024
rodlie added a commit that referenced this issue Dec 5, 2024
* Fixes #324
* Closes #359
@rodlie
Copy link
Member

rodlie commented Dec 5, 2024

Possible fix: 0b8907b

@pgilfernandez
Copy link
Author

Possible fix: 0b8907b

I'll try it tomorrow 👏🏻

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

I will merge 0b8907b today, ok?

@pgilfernandez
Copy link
Author

I will merge 0b8907b today, ok?

I'm compiling it right now, give me 15 minutes 😉

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

No rush 😄

@pgilfernandez
Copy link
Author

Tested!

If you click, that is, focus, on the canvas or extrictly the timeline area (green areas in the following screenshot) and you run Duplicate/Copy/Cut/Paste shortcut, it works as expected.

But if you try to focus on any object of the hierarchy and try to Duplicate/Copy/Cut/Paste it, it doesn't work... unless you click in the canvas and go back to the hierarchy, then any Duplicating/Copy/Cut/Paste works...

areas

Does it match with your tests?

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

But if you try to focus on any object of the hierarchy and try to Duplicate/Copy/Cut/Paste it, it doesn't work

Thanks for catching, I only tested canvas vs. keys on the timeline... meh, need to find a fix.

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

yeah... we can't use focus for shortcuts on macOS. 'keysview' (the timeline/graph) only support shortcuts for keys, not objects. Since we now block the canvas shortcuts (that supports the objects shortcuts) cmd+copy/paste/duplicate only works on keys. If we unblock the canvas we make duplicates etc.

Back to the drawing board 😄

Will probably need to catch copy/paste/dup etc in 'keysview' on macOS.

rodlie added a commit that referenced this issue Dec 10, 2024
@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

New fix: 646ea6b

@pgilfernandez
Copy link
Author

It's much better now. It feels natural...

Anyway there is still something that doesn't work as expected, if you select an object and a keyframe:

  • and "you are focused" in the canvas with the cursor in the canvas, zone 3 or 4, it only duplicates the object
    (OK, it behaves as expected)
  • and "you are focused" in the timeline with the cursor in the timeline, it only duplicates the keyframe
    (OK, it behaves as expected)
  • and "you are focused" in the timeline and move your cursor to zone 3, it only duplicates the keyframe.
    The natural behavior would be to duplicate just the object as you are in a hierarchy...
areas2

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

and "you are focused" in the timeline and move your cursor to zone 3, it only duplicates the keyframe.
The natural behavior would be to duplicate just the object as you are in a hierarchy...

Yes, any key event in zone 3 goes to zone 2 (keysview) as far as I'm aware, so if any keys are selected in zone 2 they take the key event, else the event goes to zone 1 (and the select objects take the event).

This is probably the case for Windows and Linux also (on a Mac so can't test).

@pgilfernandez
Copy link
Author

Yes, it makes sense...
Then, it would be necessary that zone 3 gets its own key event or that it could send them to zone 1 (not to zone 2 as it does now...)

What about trying this workaround, anytime you click in zone 3 (and not using shift modifier, that is, a normal click), it forces to unselect everything...

@rodlie
Copy link
Member

rodlie commented Dec 10, 2024

What about trying this workaround, anytime you click in zone 3 (and not using shift modifier, that is, a normal click), it forces to unselect everything...

That would be bad UX, I might want to adjust/browse something in zone 3 while having selected keys in zone 2.

Possible solution:

  1. add a hover state in zone 3
  2. set hover state on enter/leave events in zone 3
  3. find a way to either capture key events (this might break something else) or share hover state between zone 3 and 2, then we just check zone 3 hover state in zone 2.

EDIT: After going through the code I think sharing hover state would be the best solution.

@pgilfernandez
Copy link
Author

It sounds nice!

@rodlie
Copy link
Member

rodlie commented Dec 11, 2024

Screenshot 2024-12-11 at 17 54 46

Note that both "boxes" has the same behavior, so shortcuts on box2 will also end up where you have the focus.

So, we might need to forward all (cmd/ctrl) key events to the canvas from the boxes, else we might confuse users.

@pgilfernandez
Copy link
Author

Yes, I find it perfect this way. At the end Friction has 2 kind of elements: objects (paths, texts, texts, ellipses, ...) and keyframes, right?
What we want here is to differentiate them and just duplicate/copy/paste one kind at a time...

rodlie added a commit that referenced this issue Dec 13, 2024
1. when entering boxes widget we set focus to get key events (clear focus when leaving)
2. if hover state is true we forward key events to main window
3. main window will process supported key events for current canvas
@rodlie
Copy link
Member

rodlie commented Dec 13, 2024

Suggested UX fix: 4855bba

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working mac
Projects
None yet
Development

No branches or pull requests

3 participants