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

Feature: remember last cursor/pointer position #252

Closed
wants to merge 10 commits into from

Conversation

juarezr
Copy link
Contributor

@juarezr juarezr commented Jun 29, 2023

Proposed behavior

  • After a window receives focus, the extension:
    • Checks if the cursor already is inside the window.
    • Checks if it has the last cursor position inside the window.
    • If positive then it doesn't change the cursor position
    • Otherwise:
      • When it has the last cursor position then it moves the cursor to this position.
      • Otherwise, it moves the cursor to the center of that window.
  • Add a setting in preferences to enable it.

Advantages

Using the last position for positioning the cursor has the following advantages:

  • It's a better guess of what would be the shortest path from the cursor position to the next click position.
  • It considers users' intentions for cursor placement.

Disavantages

  • It could raise involuntary click incident rates as it puts often the cursor near the last clicked widget.
  • Sometimes users prefer fixed positions.
  • The center of the window is the shortest distance to all points inside the window, thus being better positioned for fast movement.
    • However, mouse clicks tend to concentrate on some hot spots instead of being linearly distributed through the window.

@juarezr
Copy link
Contributor Author

juarezr commented Jun 29, 2023

Fixes #227

@juarezr juarezr changed the title Feature/remember last position Feature: remember last cursor/pointer position Jun 29, 2023
@jmmaranan
Copy link
Collaborator

jmmaranan commented Jun 29, 2023

Hi @juarezr - thanks for the implementation. Besides, the move action, I think you also need to apply it on the swap. A swap is triggered by Ctrl + Super + <directional mapping - e.g. usually mapped to h,j,k,l>

@jmmaranan
Copy link
Collaborator

One issue I notice is that when in tabbed mode, and clicking on any tab, it steals the mouse focus and moves it to the center.

@juarezr
Copy link
Contributor Author

juarezr commented Jul 2, 2023

Besides, the move action, I think you also need to apply it on the swap.

I was unsure if it would move the cursor in this case.

One issue I notice is that when in tabbed mode, and clicking on any tab, it steals the mouse focus and moves it to the center.

  • It moves the cursor to the center because of the window capturing focus,
  • What should be the behavior in this case?

@jmmaranan
Copy link
Collaborator

jmmaranan commented Jul 2, 2023

I was unsure if it would move the cursor in this case.

Currently, when the windows are swapped, the active focus is on the target window not the source. So cursor, I think, should move to the target swapped window.

One issue I notice is that when in tabbed mode, and clicking on any tab, it steals the mouse focus and moves it to the center.

* It moves the cursor to the center  because of the window capturing focus,

* What should be the behavior in this case?

It should not steal focus when doing a mouse click, it is undesired behavior.

@juarezr
Copy link
Contributor Author

juarezr commented Jul 3, 2023

I think that the last changes tackle the issues raised.

There is something else that needs fixing?

@jmmaranan
Copy link
Collaborator

Please see additional comments from OP on #227. I will test it this week. Thanks

@juarezr
Copy link
Contributor Author

juarezr commented Aug 5, 2023

@jmmaranan,

Is there something pending in this PR that I could tackle?
Could you share your opinion regarding merging?

Thanks,

@jmmaranan
Copy link
Collaborator

jmmaranan commented Aug 6, 2023

Hi @juarezr - this PR seems to claim to fix #227 - however, the OP for that ticket did not seem to indicate it does. And additional follow-up from you if it was quarter-tiling (an existing mode) and binary-tiling was left unanswered. So I would say we remove that it fixes 227. I did find a closed issue #225 - that seems to be much closer to where your changes can apply. I will re-open that ticket.

Now on to the issues, looks like:

  • Tab clicking and focus stealing is now fixed. Thanks!
  • Noticed that the focus and moving the pointer sometimes does not work on newly opened windows. See screen capture.
  • Forge has a move-window [Super + Shift + direction keys or h,j,k,l] in addition to swap-window [Ctl + Shift + direction keys or h,j,k,l] - move-window does not focus the window.
Screencast.from.2023-08-06.10-22-02.webm

@jmmaranan
Copy link
Collaborator

jmmaranan commented Aug 6, 2023

Another small issue (but should not affect this PR) I am seeing is that fiddling with the mouse-focus click, between windows sometimes steals the focus. I don't think users do this normally but something about the performance of trying to focus the pointer. Tried to capture it on the screencast below:

Screencast.from.2023-08-06.11-15-06.webm

@juarezr
Copy link
Contributor Author

juarezr commented Aug 9, 2023

Wow! Awesome testing!
I'll take a look soon.

@juarezr juarezr force-pushed the feature/remember-last-position branch from c30f556 to 99f2fcd Compare August 21, 2023 04:01
@juarezr
Copy link
Contributor Author

juarezr commented Aug 21, 2023

@jmmaranan,

  • Hopefully, I fixed the focus issues in the move window shortcuts and newly opened windows.
  • I have also improved the Tab clicking handling for tackling another issue that I've found.
  • Regarding the fiddling with the mouse-focus click:
    • I couldn't reproduce it.
    • I suppose that could be a race condition while queueing the focus move event.
    • Please, let me know if it continues to happen with these latest modifications.
    • Perhaps it is also gone.

@jmmaranan
Copy link
Collaborator

I'll review it sometime over the weekend

@jmmaranan
Copy link
Collaborator

Hi @juarezr got busy on day job. Will try again this weekend.

@jmmaranan
Copy link
Collaborator

Hi @juarezr would you be able to rebase using main? Main branch has been upgraded to gnome 45 with import/export

@juarezr
Copy link
Contributor Author

juarezr commented Oct 11, 2023

Hi @jmmaranan,

I'll update it as soon as I have some free time.

@juarezr juarezr closed this Nov 1, 2023
@juarezr juarezr deleted the feature/remember-last-position branch November 1, 2023 02:42
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