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

handle resize like most window managers #189

Open
ghost opened this issue Jul 20, 2022 · 15 comments
Open

handle resize like most window managers #189

ghost opened this issue Jul 20, 2022 · 15 comments

Comments

@ghost
Copy link

ghost commented Jul 20, 2022

As the title suggests, this issue encourages the use of mod4 and right click instead of mod1 and left click.

This way this window manager would be more convenient for newbies who want to experiment.
I've come back to this window manager a few times until I noticed how to resize windows.

I've already tried to implement this, but my C knowledge is very limiting.

Keep up the good work!
Hope this gets implemented.

@JLErvin
Copy link
Owner

JLErvin commented Jul 20, 2022

imo there are two main problems here,

The first is discoverability - it should be intuitive for folks to find out how to resize windows via the documentation. We could try adding something to the man pages or docs to help with this. Ideally I think we could add some sort of functionality to allow regional-window resizing (i.e. if your cursor is at the bottom right of a window you can drag to resize it there, etc).

The second is configurability - currently we hard-code the fact that window resizing and movement via the pointer is initiated by a left-pointer click. However, we could make this configurable to the right mouse button quite easily. If you're interested in implementing this, I suggest looking at this commit for reference. I can try taking a swing at this, but I can't guarantee I'll have bandwidth anytime soon.

@ghost
Copy link
Author

ghost commented Jul 21, 2022

Documenting and changing the resize behaviour seem good suggestions.

However, I am unable to figure out how to change resize from left click to right click. Could you give me a hint, or even better, look at it yourself?

As I mentioned above, my C knowledge is not really good.

@Sidd-Dino
Copy link
Contributor

Sidd-Dino commented Jul 25, 2022

I've made an attempt to implement the feature in question in a separate branch on my fork up to date with upstream
You can modify the buttons used for moving and resizing during compile time by modifying config.h.

#define MOVE_MASK Mod4Mask
#define MOVE_BUTTON 1
#define MOVE_BUTTON_MASK Button1Mask

#define RESIZE_MASK Mod4Mask
#define RESIZE_BUTTON 3
#define RESIZE_BUTTON_MASK Button3Mask

Feel free to check out the commit history to see what I changed

P.S: Button3 is the right mouse button

@letterlock
Copy link

to add to this, I've merged @Sidd-Dino's changes with #184 on my own fork here.

i'm not entirely sure if i've done everything correctly (i wasn't sure if i could merge branches across forks), and i'm pretty sure both of these changes already have a pr associated with them, so i've just grouped them together for convenience if anyone stumbles in here while we wait for our based @JLErvin to return to us. 🙏

@Sidd-Dino
Copy link
Contributor

Hey @letterlock,
The changes I made in my fork are a bit janky..
They were intended as a proof of concept.
I might make changes to them so that they are... less jank.

@letterlock
Copy link

by all means! i just wanted easier access to a stable berry build with your fix and #184 together so i didn't have to manually rebuild it if i needed to reinstall etc.

i don't know enough c to be able to recognise janky code, let alone run with it and build other stuff on top of it, if that was your concern.

@JLErvin
Copy link
Owner

JLErvin commented Aug 21, 2022

I encourage you to open a PR for this @Sidd-Dino, that's the most effective way for me to give feedback so that we can merge this into the main repo (btw, you can create PRs for forks).

In general I think it's pretty close so I think it could be worth doing :)

@Sidd-Dino
Copy link
Contributor

#191
A new PR is born.
The new PR has modifications that allows configuration during compile time (via config.h) and during runtime using berryc.

The config.h options being

#define MOVE_BUTTON   1
#define RESIZE_BUTTON 1

The berryc options being

berryc move_button    1
berryc resize_button  3

@letterlock
Copy link

that looks great. is there any way i can help test these changes (if necessary)? how much does the code differ from your fork?

@Sidd-Dino
Copy link
Contributor

Just try running it and see if moving/resizing windows with the configured mouse buttons works or not.

configure the mouse buttons using berryc

Mouse buttons
1 - Left Mouse Button
2 - Middle Mouse Button (Pressing down on the scroll wheel)
3 - Right Mouse Button

The branch mentioned in the PR differs greatly from the branch i mentioned previously in this thread.
The IPC stuff is just stuff needed for berryc to communicate with berry

@letterlock
Copy link

sweet. tested with

berryc resize_mask "mod4"
berryc resize_button 3

in my autostart file. works as expected. fiddling around with different buttons seems also to work fine, no responding to buttons outside of the specified commands or not responding to the correct ones.

@letterlock
Copy link

so - unsure if i should be double posting here or editing my earlier reply, so sorry if this is bad form.

it seems that this build breaks the ability to drag windows by the title bar of their decorations. it's a very minor issue imo and i only notice it because i tend to swap between my gaming rig and my linux box quite often. i just figured i would mention it, as it seems related to @Sidd-Dino's changes.

@Sidd-Dino
Copy link
Contributor

Whooops my bad.
Working on fixing it ASAP

@Sidd-Dino
Copy link
Contributor

I've made a quick and dirty change to the line mentioned below

From this

if (state == (unsigned)conf.move_mask && bev->button == (unsigned)conf.move_button ) {

To this

if ((state == (unsigned)conf.move_mask && bev->button == (unsigned)conf.move_button) || ev.xbutton.state == Button1Mask) {

Does this change bring forth the desired behaviour?

P.S : Please mention any new issues caused by my changes in this PR #191. It's easier to keep track of things :D

@letterlock
Copy link

that does indeed fix it.

P.S : Please mention any new issues caused by my changes in this PR #191. It's easier to keep track of things :D

right, of course! will do that from now on. i knew there was a better place but i was blanking. x^)

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

No branches or pull requests

3 participants