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

feat: add a config to control which window to focus on when closing window #7368

Merged
merged 7 commits into from
Aug 21, 2024

Conversation

ParaN3xus
Copy link
Contributor

Describe your PR, what does it fix/add?

This PR introduces a new configuration option, input::focus_on_close, which controls the window focus behavior after a window is closed.

Given the existing focus-related settings within the input category, I have placed this configuration there as well.

input::focus_on_close is an integer, and currently supports two values:

  • 0: Maintains the current behavior, where focus shifts to the next window candidate as determined by getNextWindowCandidate.
  • 1: Changes focus to the window under the cursor, which I believe offers a more intuitive user experience.

The reason for using an integer instead of a boolean is to allow for future extensibility. For example, the option could be extended to focus on the previously focused window, or implement other behaviors, depending on user needs or future contributions.

Is there anything you want to mention? (unchecked code, possible bugs, found problems, breaking compatibility, etc.)

No known issues.

Is it ready for merging, or does it need work?

Yes, it is ready for merging.

@vdawg-git
Copy link
Member

This is quite cool. I am not a maintainer nor know C++, but I think using a string like "cursor" and "next" instead of an integer is nicer to understand for the end-user :D

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-format and wiki mr needed :)

@vaxerski
Copy link
Member

This is quite cool. I am not a maintainer nor know C++, but I think using a string like "cursor" and "next" instead of an integer is nicer to understand for the end-user :D

pls dont

@vdawg-git
Copy link
Member

Pf, strings are nicer

@ParaN3xus
Copy link
Contributor Author

Apologies for missing the PR guidelines. I've now applied clang-format and updated the wiki(hyprland-wiki/pull/753). Thank you for the feedback!

@ParaN3xus
Copy link
Contributor Author

@Visual-Dawg
I considered using strings, but chose integers to align with the existing configurations in the project.

Settings like follow_mouse, off_window_axis_events, and emulate_discrete_scrollall use integers to represent different modes of operation. These settings are functionally similar in that they each control various behavioral outcomes.

I agree that strings can be more user-friendly, but in this case, I prioritized consistency with existing configurations.

@vaxerski
Copy link
Member

strings will be parsable in the future once the descriptions branch gets merged (I should merge it)

@vaxerski
Copy link
Member

vaxerski commented Aug 17, 2024

ye I merged #7377 can you add your opt to configDescriptions.hpp too? (rebase on main first)

@ParaN3xus
Copy link
Contributor Author

I've completed the rebase and updated ConfigDescriptions.hpp. Please review the latest changes.
@vaxerski

Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rest ok

src/config/ConfigDescriptions.hpp Outdated Show resolved Hide resolved
src/config/ConfigDescriptions.hpp Outdated Show resolved Hide resolved
vaxerski
vaxerski previously approved these changes Aug 20, 2024
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the patience!

@vaxerski
Copy link
Member

clang-format needed

@vaxerski vaxerski merged commit 946ed1f into hyprwm:main Aug 21, 2024
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants