TWin: Improve Active Screen Borders, add Active Corners support and Aerosnap #331

Merged
blu.256 merged 1 commits from feat/twin-electric-improvements into master 11 months ago
blu.256 commented 1 year ago
Collaborator

This PR is intended to improve the existing "Active Borders" feature of TWin.

Right now, when you activate "Active Borders", you can switch between desktops by pushing your mouse cursor against one of the desktop's four borders. The two options you have (apart from disabling the feature) is to do this always or only when you are moving a window. The option is accessible in the "Window Behaviour" options dialog, "Advanced" tab.

This PR:

  1. Backports a cleaner implementation of this feature from the KDE 4.0 version of KWin. It works exactly the same, if not a little faster (might be just my impression), but the code is better maintainable and less repetitive. It also provides us with electric corners support, although it is not used yet (but we might make that user-configurable). This introduces a couple of public API changes in the Client class, but I don't think this will break any applications.

  2. Building on the previous changes, I have backported the quick tiling feature (also known as aero-snapping), which allows you to maximize a window to half of the screen by dragging it into a screen border (an option to maximize window when dragging the window towards the top border is also available). So the "active border" feature gets a new mode, the quick tiling feature, as seen in many popular DE's out there.

  3. Adds an option to automatically restore original window size when dragging a tiled or maximized window.

  4. Brings the tiling behaviour closer to the behaviour of existing TDE features.

  5. Updates/adds documentation for all the above changes.

I have marked R14.2.0 as possible milestone, but if you think it can be merged safely in a maintenance release, then it could go into R14.1.1.

Note: electric border is the TWin/old KWin term for active border. We get rid of that in this PR.

Known bugs

  • Doesn't work with wireframe windows yet (the kind of helper windows that appear instead of a window when you move that window while window updates are disabled for moving windows. This option can be set in the "Movement" tab of the "Window behaviour" dialog). FIXED
  • Regression: active borders don't change the mouse cursor to an arrow pointing towards the corresponding edge. Might be implemented later.
  • See Michele's comments below. FIXED
This PR is intended to improve the existing "**Active Borders**" feature of TWin. Right now, when you activate "Active Borders", you can switch between desktops by pushing your mouse cursor against one of the desktop's four borders. The two options you have (apart from disabling the feature) is to do this always or only when you are moving a window. The option is accessible in the "Window Behaviour" options dialog, "Advanced" tab. This PR: 1. Backports a cleaner implementation of this feature from the KDE 4.0 version of KWin. It works exactly the same, if not a little faster (might be just my impression), but the code is better maintainable and less repetitive. It also provides us with electric *corners* support, although it is not used yet (but we might make that user-configurable). This introduces a couple of public API changes in the Client class, but I don't think this will break any applications. 2. Building on the previous changes, I have backported the quick tiling feature (also known as aero-snapping), which allows you to maximize a window to half of the screen by dragging it into a screen border (an option to maximize window when dragging the window towards the top border is also available). So the "active border" feature gets a new mode, the quick tiling feature, as seen in many popular DE's out there. 3. Adds an option to automatically restore original window size when dragging a tiled or maximized window. 4. Brings the tiling behaviour closer to the behaviour of existing TDE features. 5. Updates/adds documentation for all the above changes. ~~I have marked R14.2.0 as possible milestone, but if you think it can be merged safely in a maintenance release, then it could go into R14.1.1.~~ Note: *electric border* is the TWin/old KWin term for *active border*. We get rid of that in this PR. ## Known bugs * ~~Doesn't work with wireframe windows yet (the kind of helper windows that appear instead of a window when you move that window while window updates are disabled for moving windows. This option can be set in the "Movement" tab of the "Window behaviour" dialog).~~ FIXED * Regression: active borders don't change the mouse cursor to an arrow pointing towards the corresponding edge. Might be implemented later. * ~~See Michele's comments below.~~ FIXED
blu.256 added this to the R14.2.0 release milestone 1 year ago
blu.256 changed title from TWin: Active Screen Borders and Corners to TWin: Improve Active Screen Borders, add Active Corners support and Aerosnap 1 year ago
blu.256 force-pushed feat/twin-electric-improvements from 4db99653fe to 52c7661129 1 year ago
blu.256 commented 1 year ago
Poster
Collaborator

How to test:

  • After building successfully you will have to restart TWin (twin --replace & will do)
  • Go to TCC > Desktop > Window Behaviour > Advanced
  • Change Active/Electric Border options.
  • Press apply
  • Try pushing your mouse cursor against a screen border. The behaviour should match the settings.
  • Try the same thing, only this time drag a window there. The behaviour once again should match the settings.
How to test: * After building successfully you will have to restart TWin (`twin --replace &` will do) * Go to TCC > Desktop > Window Behaviour > Advanced * Change Active/Electric Border options. * Press apply * Try pushing your mouse cursor against a screen border. The behaviour should match the settings. * Try the same thing, only this time drag a window there. The behaviour once again should match the settings.
blu.256 force-pushed feat/twin-electric-improvements from 52c7661129 to 76a08deba3 1 year ago
Owner

Building on the previous changes, I have backported the quick tiling feature (also known as aero-snapping), which allows you to maximize a window to half of the screen by dragging it into a screen border (an option to maximize window when dragging the window towards the top border is also available). So the "electric border" feature gets a new mode, the quick tiling feature, as seen in many popular DE's out there.

THANK YOU Philippe!!!
This is something I really missed from Windows and that I wanted to implement at some point.

I will test it next week after R14.1.0 is hard frozen and provide feedback (== nagging issue list :-) ) as usual. But wanted to say thanks in advance for this feature!

> Building on the previous changes, I have backported the quick tiling feature (also known as aero-snapping), which allows you to maximize a window to half of the screen by dragging it into a screen border (an option to maximize window when dragging the window towards the top border is also available). So the "electric border" feature gets a new mode, the quick tiling feature, as seen in many popular DE's out there. THANK YOU Philippe!!! This is something I really missed from Windows and that I wanted to implement at some point. I will test it next week after R14.1.0 is hard frozen and provide feedback (== nagging issue list :-) ) as usual. But wanted to say thanks in advance for this feature!
Owner

One initial feedback I can alrady provide is that in TCC, the electric border is called "Active border", so it may be good to use the same terminology in the commit message and rename all the electric* stuff to active* in the code. What do you think?

One initial feedback I can alrady provide is that in TCC, the electric border is called "Active border", so it may be good to use the same terminology in the commit message and rename all the `electric*` stuff to `active*` in the code. What do you think?
blu.256 commented 1 year ago
Poster
Collaborator

@MicheleC

One initial feedback I can alrady provide is that in TCC, the electric border is called "Active border", so it may be good to use the same terminology in the commit message and rename all the electric* stuff to active* in the code. What do you think?

I don't see why not. Consistent terminology would be good and "active" better fits the description of the feature, as I plan to add user-configurable actions support for both borders and corners.

@MicheleC >One initial feedback I can alrady provide is that in TCC, the electric border is called "Active border", so it may be good to use the same terminology in the commit message and rename all the `electric*` stuff to `active*` in the code. What do you think? I don't see why not. Consistent terminology would be good and "active" better fits the description of the feature, as I plan to add user-configurable actions support for both borders and corners.
Owner

Great!
I also have a question: do we currently need to touch the border exactly or is there a band of pixels fom the border within which the "active" functionality works? Like for example if the mouse is within 10 pixed from the border, that is it consider as being "active"? Similar to what we have for windows snapping to border functinoality.

The question is important if you use TDE inside a VM in non fullscreen mode, since being exactly on the border could be a bit challenging.

Great! I also have a question: do we currently need to touch the border exactly or is there a band of pixels fom the border within which the "active" functionality works? Like for example if the mouse is within 10 pixed from the border, that is it consider as being "active"? Similar to what we have for windows snapping to border functinoality. The question is important if you use TDE inside a VM in non fullscreen mode, since being exactly on the border could be a bit challenging.
blu.256 commented 1 year ago
Poster
Collaborator

do we currently need to touch the border exactly or is there a band of pixels fom the border within which the "active" functionality works? Like for example if the mouse is within 10 pixed from the border, that is it consider as being "active"? Similar to what we have for windows snapping to border functinoality.

No, you actually have to "push" into the border for a bit to activate the functionality. I suppose this is to prevent accidental activation.

The question is important if you use TDE inside a VM in non fullscreen mode, since being exactly on the border could be a bit challenging.

For this case we could add some other way of activation, e.g. options in the window menu?

> do we currently need to touch the border exactly or is there a band of pixels fom the border within which the "active" functionality works? Like for example if the mouse is within 10 pixed from the border, that is it consider as being "active"? Similar to what we have for windows snapping to border functinoality. No, you actually have to "push" into the border for a bit to activate the functionality. I suppose this is to prevent accidental activation. > The question is important if you use TDE inside a VM in non fullscreen mode, since being exactly on the border could be a bit challenging. For this case we could add some other way of activation, e.g. options in the window menu?
Owner

No, you actually have to "push" into the border for a bit to activate the functionality. I suppose this is to prevent accidental activation.

Currently there is a timer that can be set, so you have to push against the border for the set time before active border changes desktop (for example).

For this case we could add some other way of activation, e.g. options in the window menu?

maybe add a keyboard key that needs to be hold down to activate active border, like for example Shift+mouse near border or something like that. Anyway we can work on this later, after this PR is merged.

> No, you actually have to "push" into the border for a bit to activate the functionality. I suppose this is to prevent accidental activation. Currently there is a timer that can be set, so you have to push against the border for the set time before active border changes desktop (for example). > For this case we could add some other way of activation, e.g. options in the window menu? maybe add a keyboard key that needs to be hold down to activate active border, like for example Shift+mouse near border or something like that. Anyway we can work on this later, after this PR is merged.
Owner

Hi Philippe,
I did a first round of testing. Love the functionality and I have several ideas for improvement after we merge this PR. But first, feedback on testing and things to look at.

  1. (minor one) In the TCC --> Desktop --> Window Behaviour --> Advanced, the "Desktop switch delay" bar is now much narrower. Although I like it more this way, it is the only one different from the others (which extend till the right side edge), therefore it seems out of place. Also being shorter, it makes moving the slider more sensitive compared to before.
    We can have a separate discussion with Slavek on whether we want to make all the sliders shorter in TCC, but that would be a different issue/PR.

  2. would be good to have a slider to set the delay required for the "tile" functionality, so that a user can set its preferred waiting time.

  3. enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Press "Esc" or right click or middle click with the mouse. I was expecting this to abort the tiling operation, but instead it completes it as if I had released the left mouse button (which I am still holding down).
    Not sure if this is something caused by testing inside a VM, can you test and let me know?

  4. enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Move the mouse back away from the border without releasing the left mouse button. I was expecting the "half screen" empty window border to disappear since now the mouse is no longer close to the desktop border, but instead it didn't. Or better if I am very very careful and fast to move to-and-away from the border, the "half screen" empty window border disappear. But if I hover on the border just a fraction of time longer, then it seems to get locked and does not disappear.
    Again, not sure if this is something caused by testing inside a VM, can you test and let me know?

  5. "tile" functionality should not work for windows which cannot be maximized or resized IMO. Try KCalc and you will see that tiling it will result in a not so nice window size. Note how KCalc cannot be manually resized/maximized

  6. if you right or middle click on the maximize button of a Konsole window, the window will be maximized in either horizontal or vertical direction (depending on your settings). Dragging the window title bar will move the window but without resizing it to the original size.
    The "tile" functionality is not consistent with this behavior. After a window got tiled, dragging the window title bar will result in the window going back to its original size before tiling.
    For the time being I think we should make "tiling" consistent with the right/middle click maximize behavior. We can later add an option for both functionalities to let the user choose whether to keep the current size or go back to its original size when a window gets dragged, but that will be part of the enhancement follow up.

Overall, great enhancement and expect several requests for additional functionality after this PR is merged :-)
I can setup testing on a real laptop if needed, in case we are not sure whether testing in a VM causes any strange behaviors

Hi Philippe, I did a first round of testing. Love the functionality and I have several ideas for improvement after we merge this PR. But first, feedback on testing and things to look at. 1. (minor one) In the TCC --> Desktop --> Window Behaviour --> Advanced, the "Desktop switch delay" bar is now much narrower. Although I like it more this way, it is the only one different from the others (which extend till the right side edge), therefore it seems out of place. Also being shorter, it makes moving the slider more sensitive compared to before. We can have a separate discussion with Slavek on whether we want to make all the sliders shorter in TCC, but that would be a different issue/PR. 2. would be good to have a slider to set the delay required for the "tile" functionality, so that a user can set its preferred waiting time. 3. enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Press "Esc" or right click or middle click with the mouse. I was expecting this to abort the tiling operation, but instead it completes it as if I had released the left mouse button (which I am still holding down). Not sure if this is something caused by testing inside a VM, can you test and let me know? 4. enable "tile" functionality. Drag a Konsole window to the right border. The "half screen" empty window border gets displayed. Move the mouse back away from the border without releasing the left mouse button. I was expecting the "half screen" empty window border to disappear since now the mouse is no longer close to the desktop border, but instead it didn't. Or better if I am very very careful and fast to move to-and-away from the border, the "half screen" empty window border disappear. But if I hover on the border just a fraction of time longer, then it seems to get locked and does not disappear. Again, not sure if this is something caused by testing inside a VM, can you test and let me know? 5. "tile" functionality should not work for windows which cannot be maximized or resized IMO. Try KCalc and you will see that tiling it will result in a not so nice window size. Note how KCalc cannot be manually resized/maximized 6. if you right or middle click on the maximize button of a Konsole window, the window will be maximized in either horizontal or vertical direction (depending on your settings). Dragging the window title bar will move the window but *without* resizing it to the original size. The "tile" functionality is not consistent with this behavior. After a window got tiled, dragging the window title bar will result in the window going back to its original size before tiling. For the time being I think we should make "tiling" consistent with the right/middle click maximize behavior. We can later add an option for both functionalities to let the user choose whether to keep the current size or go back to its original size when a window gets dragged, but that will be part of the enhancement follow up. Overall, great enhancement and expect several requests for additional functionality after this PR is merged :-) I can setup testing on a real laptop if needed, in case we are not sure whether testing in a VM causes any strange behaviors
Owner

Another issue related to point 6. from previous post is the following. Tile a window to one side (for example right side). Then grab the window border to resize it (for example use the left border). A user would expect the window to expand as it usually happens, but again the window goes back to its original size, which is very annoying.

Another issue related to point 6. from previous post is the following. Tile a window to one side (for example right side). Then grab the window border to resize it (for example use the left border). A user would expect the window to expand as it usually happens, but again the window goes back to its original size, which is very annoying.
blu.256 commented 1 year ago
Poster
Collaborator

Hello Michele, thank you for the thorough testing. Unfortunately I'm somewhat busy these days so this PR will have to wait a little. I'll get back to it once I have some time on my hands.

I'd like, as part of this PR, to also implement a way to assign actions to active corners, so it might be a while until the PR is merged. I'll give priority to fixing the existing issues first, of course.

Hello Michele, thank you for the thorough testing. Unfortunately I'm somewhat busy these days so this PR will have to wait a little. I'll get back to it once I have some time on my hands. I'd like, as part of this PR, to also implement a way to assign actions to active corners, so it might be a while until the PR is merged. I'll give priority to fixing the existing issues first, of course.
Owner

No problem Philippe, that your time. I also have a list of improvements I would like to implement, like for example the ability to tile a window to a quarter of the screen, rather than half.
I suggest we take it step by step. For example we fix up and merge this PR with the current functionality. Then we progressivily add improvements, like the one you mentioned and those that I have in mind.

No problem Philippe, that your time. I also have a list of improvements I would like to implement, like for example the ability to tile a window to a quarter of the screen, rather than half. I suggest we take it step by step. For example we fix up and merge this PR with the current functionality. Then we progressivily add improvements, like the one you mentioned and those that I have in mind.
blu.256 force-pushed feat/twin-electric-improvements from 76a08deba3 to 1e67f9971f 12 months ago
Poster
Collaborator

Issues 1, 2, 3 resolved.

Issues 1, 2, 3 resolved.
Poster
Collaborator

Issues 4, 5 resolved.

The "tile" functionality is not consistent with this behavior. After a window got tiled, dragging the window title bar will result in the window going back to its original size before tiling.

Well, it is consistent with KWin and my experience with other window managers.

For the time being I think we should make "tiling" consistent with the right/middle click maximize behavior. We can later add an option for both functionalities to let the user choose whether to keep the current size or go back to its original size when a window gets dragged, but that will be part of the enhancement follow up.

Agreed. I'll just comment out the relevant code for now. I prefer this behaviour, though, and think that it should be the same for maximized windows, too.

Issues 4, 5 resolved. > The "tile" functionality is not consistent with this behavior. After a window got tiled, dragging the window title bar will result in the window going back to its original size before tiling. Well, it is consistent with KWin and my experience with other window managers. > For the time being I think we should make "tiling" consistent with the right/middle click maximize behavior. We can later add an option for both functionalities to let the user choose whether to keep the current size or go back to its original size when a window gets dragged, but that will be part of the enhancement follow up. Agreed. I'll just comment out the relevant code for now. I prefer this behaviour, though, and think that it should be the same for maximized windows, too.
Owner

Great, I will do a new test this week and feedback as usual.

Great, I will do a new test this week and feedback as usual.
Owner

Hi Philippe,
I did a new round of testing.

a. referring to items 1. and 2., I assume the "Border activation delay" is now common to "Switch desktop" and "Tile window", depending on which one is selected. Am I right? If that is the case, it is fine.
Aestetichally, I suggest a few tweaks. First add a title label to the 3 radio buttons, something like "Function". Second, add a small vertical spacer before "Border activation delay" label. Third: move "Hide utility window..." checkbox inside a groupbox, to make it similar to the rest of the page. What do you think?

b. regarding item 3: ESC function is now ok. The function of right click is also ok as you implemented it. Initially I thought we should abort the move, but other drag-n-drop operations work in a similar way with right click mouse button completing the move.

c. item 4: (this may be caused by screen refreshing within my VM, so please read and double check on your side). There is improvement but it still need more work, IMO. When the mouse touches the border, the black "half screen" border shows up. If we move the mouse away and release the button, the border disappear but only after releasing the mouse button. If we don't release the button, the border stays there all the time. Moreover if we move the window far enough before releaseing the border, it seems the "black border" does some funny things (please check and let me know if it happens on a real pc too, or if it is just my VM playing up)

d. item 5: ok

e. item 6: still not ok. After tiling a window and dragging/resizing it, the window still goes back to the original size before tiling.

In summary:

  • items 2, 3 and 5 ok
  • item 1: suggested some minor eastetichal improvements
  • item 4: test and confirm behaviour on real machine. May need some update to hide the "black border" as soon as the mouse exits the activation zone
  • item 6: still not ok

PS: I am already using this functionality daily, even with the mentioned issues. What a great one!

Hi Philippe, I did a new round of testing. a. referring to items 1. and 2., I assume the "Border activation delay" is now common to "Switch desktop" and "Tile window", depending on which one is selected. Am I right? If that is the case, it is fine. Aestetichally, I suggest a few tweaks. First add a title label to the 3 radio buttons, something like "Function". Second, add a small vertical spacer before "Border activation delay" label. Third: move "Hide utility window..." checkbox inside a groupbox, to make it similar to the rest of the page. What do you think? b. regarding item 3: ESC function is now ok. The function of right click is also ok as you implemented it. Initially I thought we should abort the move, but other drag-n-drop operations work in a similar way with right click mouse button completing the move. c. item 4: (this may be caused by screen refreshing within my VM, so please read and double check on your side). There is improvement but it still need more work, IMO. When the mouse touches the border, the black "half screen" border shows up. If we move the mouse away and release the button, the border disappear but only after releasing the mouse button. If we don't release the button, the border stays there all the time. Moreover if we move the window far enough before releaseing the border, it seems the "black border" does some funny things (please check and let me know if it happens on a real pc too, or if it is just my VM playing up) d. item 5: ok e. item 6: still not ok. After tiling a window and dragging/resizing it, the window still goes back to the original size before tiling. In summary: - items 2, 3 and 5 ok - item 1: suggested some minor eastetichal improvements - item 4: test and confirm behaviour on real machine. May need some update to hide the "black border" as soon as the mouse exits the activation zone - item 6: still not ok PS: I am already using this functionality daily, even with the mentioned issues. What a great one!
Poster
Collaborator

Hello, Michele! Let's see what we have here...

a. referring to items 1. and 2., I assume the "Border activation delay" is now common to "Switch desktop" and "Tile window", depending on which one is selected. Am I right? If that is the case, it is fine.

Yes, it is. Code-wise it already was, BTW.

Aestetichally, I suggest a few tweaks. First add a title label to the 3 radio buttons, something like "Function". Second, add a small vertical spacer before "Border activation delay" label. Third: move "Hide utility window..." checkbox inside a groupbox, to make it similar to the rest of the page. What do you think?

I agree, and actually I wanted to propose that the active border settings be moved into a separate tab, seeing that I'd like to add configurable active borders in next PR. I'll make the changes you requested, but before creating that separate tab I want your feedback first.

c. item 4: (this may be caused by screen refreshing within my VM, so please read and double check on your side). There is improvement but it still need more work, IMO. When the mouse touches the border, the black "half screen" border shows up. If we move the mouse away and release the button, the border disappear but only after releasing the mouse button. If we don't release the button, the border stays there all the time. Moreover if we move the window far enough before releaseing the border, it seems the "black border" does some funny things (please check and let me know if it happens on a real pc too, or if it is just my VM playing up)

I can reproduce this. I think it has something to do with 3d38021d65 because it behaves correctly when you uncheck "Show contents when moving windows" in the "Moving" tab.

e. item 6: still not ok. After tiling a window and dragging/resizing it, the window still goes back to the original size before tiling.

I haven't fixed that one yet `:)

PS: I am already using this functionality daily, even with the mentioned issues. What a great one!

Great! I also use it daily and it's a great time saver :)

Hello, Michele! Let's see what we have here... > a. referring to items 1. and 2., I assume the "Border activation delay" is now common to "Switch desktop" and "Tile window", depending on which one is selected. Am I right? If that is the case, it is fine. Yes, it is. Code-wise it already was, BTW. > Aestetichally, I suggest a few tweaks. First add a title label to the 3 radio buttons, something like "Function". Second, add a small vertical spacer before "Border activation delay" label. Third: move "Hide utility window..." checkbox inside a groupbox, to make it similar to the rest of the page. What do you think? I agree, and actually I wanted to propose that the active border settings be moved into a separate tab, seeing that I'd like to add configurable active borders in next PR. I'll make the changes you requested, but before creating that separate tab I want your feedback first. > c. item 4: (this may be caused by screen refreshing within my VM, so please read and double check on your side). There is improvement but it still need more work, IMO. When the mouse touches the border, the black "half screen" border shows up. If we move the mouse away and release the button, the border disappear but only after releasing the mouse button. If we don't release the button, the border stays there all the time. Moreover if we move the window far enough before releaseing the border, it seems the "black border" does some funny things (please check and let me know if it happens on a real pc too, or if it is just my VM playing up) I can reproduce this. I think it has something to do with 3d38021d65 because it behaves correctly when you uncheck "Show contents when moving windows" in the "Moving" tab. > e. item 6: still not ok. After tiling a window and dragging/resizing it, the window still goes back to the original size before tiling. I haven't fixed that one yet `:) > PS: I am already using this functionality daily, even with the mentioned issues. What a great one! Great! I also use it daily and it's a great time saver :)
blu.256 force-pushed feat/twin-electric-improvements from 3d38021d65 to 834d1988e7 12 months ago
Poster
Collaborator

I've made the suggested aesthetical improvements in the options UI and resolved item 6 by adding an option to turn restoring original size on/off and preventing restoring the original size when resizing the window.

I've made the suggested aesthetical improvements in the options UI and resolved item 6 by adding an option to turn restoring original size on/off and preventing restoring the original size when resizing the window.
Owner

Great, will do another test either tomorrow or Wednesday and feedback.

I wanted to propose that the active border settings be moved into a separate tab

Sounds good to me, especially if you already have some ideas in mind for things to add.

Great, will do another test either tomorrow or Wednesday and feedback. > I wanted to propose that the active border settings be moved into a separate tab Sounds good to me, especially if you already have some ideas in mind for things to add.
Owner

Or maybe move the "Shading" part under the "Titlebar Actions" tab and keep the "Advanced" tab for the active border? What do you think? Otherwise the "Andanced" tab would look pretty empty and almost pointless.

Or maybe move the "Shading" part under the "Titlebar Actions" tab and keep the "Advanced" tab for the active border? What do you think? Otherwise the "Andanced" tab would look pretty empty and almost pointless.
Owner

I've made the suggested aesthetical improvements in the options UI and resolved item 6 by adding an option to turn restoring original size on/off and preventing restoring the original size when resizing the window.

Thanks Philippe. I did a new test.

Item 1: the eastetichal changes look good, thanks. Just one you missed is moving "Hide utility window..." checkbox inside a groupbox. The reason for asking this is that with QtCurve theme there is no border around a group box, so that checkbox looks out of alignment and also it is not clear that it is not related to the "active border" settings. Let me know if you need a screenshot of what it looks like.

Item 6: great, it works well with the new checkbox. That is a good improvement, thanks. As future enhancement (after this PR) it may be worth adding the possibility to control the behavior with a modifier key as well, but we can discuss this in future

I can reproduce this. I think it has something to do with 3d38021d65 because it behaves correctly when you uncheck "Show contents when moving windows" in the "Moving" tab.

Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible?

Other than the points above, the PR seems to be mostly ready (haven't looked at the code yet though).

> I've made the suggested aesthetical improvements in the options UI and resolved item 6 by adding an option to turn restoring original size on/off and preventing restoring the original size when resizing the window. Thanks Philippe. I did a new test. Item 1: the eastetichal changes look good, thanks. Just one you missed is moving "Hide utility window..." checkbox inside a groupbox. The reason for asking this is that with QtCurve theme there is no border around a group box, so that checkbox looks out of alignment and also it is not clear that it is not related to the "active border" settings. Let me know if you need a screenshot of what it looks like. Item 6: great, it works well with the new checkbox. That is a good improvement, thanks. As future enhancement (after this PR) it may be worth adding the possibility to control the behavior with a modifier key as well, but we can discuss this in future > I can reproduce this. I think it has something to do with 3d38021d65 because it behaves correctly when you uncheck "Show contents when moving windows" in the "Moving" tab. Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible? Other than the points above, the PR seems to be mostly ready (haven't looked at the code yet though).
Poster
Collaborator

Or maybe move the "Shading" part under the "Titlebar Actions" tab and keep the "Advanced" tab for the active border? What do you think? Otherwise the "Andanced" tab would look pretty empty and almost pointless.

The Advanced tab actually is pointless, as:

  • the Shading groupbox can be moved under Titlebar Actions, as you said
  • the Hide utility window... checkbox can be placed under the Focus tab and
  • Active Border needs to be a tab on its own

So I'll try to do exactly that.

moving "Hide utility window..." checkbox inside a groupbox.

Will do.

As future enhancement (after this PR) it may be worth adding the possibility to control the behavior with a modifier key as well, but we can discuss this in future

That's a really nice idea!

Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible?

Strange, I have already pushed a fix that works for me. Have you re-checked with "Display contents in moving windows" on?

Other than the points above, the PR seems to be mostly ready (haven't looked at the code yet though).

Do you think it should target R14.2.0 or the next maintenance releaes? I've assumed the former, but it shouldn't break anything as nothing changed in the public APIs.

> Or maybe move the "Shading" part under the "Titlebar Actions" tab and keep the "Advanced" tab for the active border? What do you think? Otherwise the "Andanced" tab would look pretty empty and almost pointless. The Advanced tab actually is pointless, as: - the Shading groupbox can be moved under Titlebar Actions, as you said - the Hide utility window... checkbox can be placed under the Focus tab and - Active Border needs to be a tab on its own So I'll try to do exactly that. > moving "Hide utility window..." checkbox inside a groupbox. Will do. > As future enhancement (after this PR) it may be worth adding the possibility to control the behavior with a modifier key as well, but we can discuss this in future That's a really nice idea! > Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible? Strange, I have already pushed a fix that works for me. Have you re-checked with "Display contents in moving windows" on? > Other than the points above, the PR seems to be mostly ready (haven't looked at the code yet though). Do you think it should target R14.2.0 or the next maintenance releaes? I've assumed the former, but it shouldn't break anything as nothing changed in the public APIs.
Owner

The Advanced tab actually is pointless, as:

  • the Shading groupbox can be moved under Titlebar Actions, as you said
  • the Hide utility window... checkbox can be placed under the Focus tab and
  • Active Border needs to be a tab on its own

So I'll try to do exactly that.

Sounds good to me. Do you want to do this as part of this PR or separately after we merge this one?

Item 4: I confirm that if "Display contents in moving windows" is
disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible?

Strange, I have already pushed a fix that works for me. Have you re-checked with "Display contents in moving windows" on?

I must have made a mistake during testing yesterday, because today it is working fine.. sorry for the wrong reporting. So item 4 is also cleared.

Do you think it should target R14.2.0 or the next maintenance releaes? I've assumed the former, but it shouldn't break anything as nothing changed in the public APIs.

it depends on whether there are API/ABI changes or not. If there aren't any changes to that, we can add to R14.1.x since it is a great feature. If it changes API/ABI, we shall target R14.2.0.
Once the PR is ready, I will go through the code and feedback on this

> The Advanced tab actually is pointless, as: > - the Shading groupbox can be moved under Titlebar Actions, as you said > - the Hide utility window... checkbox can be placed under the Focus tab and > - Active Border needs to be a tab on its own > > So I'll try to do exactly that. Sounds good to me. Do you want to do this as part of this PR or separately after we merge this one? > > Item 4: I confirm that if "Display contents in moving windows" is disabled, the behavior is fine. Would be good to make it to work fine even when that is enabled though. Do you think that is feasible? > > Strange, I have already pushed a fix that works for me. Have you re-checked with "Display contents in moving windows" on? I must have made a mistake during testing yesterday, because today it is working fine.. sorry for the wrong reporting. So item 4 is also cleared. > Do you think it should target R14.2.0 or the next maintenance releaes? I've assumed the former, but it shouldn't break anything as nothing changed in the public APIs. it depends on whether there are API/ABI changes or not. If there aren't any changes to that, we can add to R14.1.x since it is a great feature. If it changes API/ABI, we shall target R14.2.0. Once the PR is ready, I will go through the code and feedback on this
Poster
Collaborator

Do you want to do this as part of this PR or separately after we merge this one?

Probably best to keep that for the next PR where I will be adding configurable actions for Active Corners.

I must have made a mistake during testing yesterday, because today it is working fine.. sorry for the wrong reporting. So item 4 is also cleared.

It's okay. Nice to hear that one's solved, too :)

it depends on whether there are API/ABI changes or not.

There are only changes internal to TWin and not exposed in public headers.

BTW, just noticed that due to Electric* -> Active* renaming some configuration options have been renamed:

  • ElectricBorder -> ActiveBorder
  • ElectricBorderDelay -> ActiveBorderDelay

Option values of ActiveBorder are compatible with old option values of ElectricBorder. Considering this, it might make sense either to revert renaming in this case or automatically convert these options using the release script.

> Do you want to do this as part of this PR or separately after we merge this one? Probably best to keep that for the next PR where I will be adding configurable actions for Active Corners. > I must have made a mistake during testing yesterday, because today it is working fine.. sorry for the wrong reporting. So item 4 is also cleared. It's okay. Nice to hear that one's solved, too :) > it depends on whether there are API/ABI changes or not. There are only changes internal to TWin and not exposed in public headers. BTW, just noticed that due to Electric* -> Active* renaming some configuration options have been renamed: * ElectricBorder -> ActiveBorder * ElectricBorderDelay -> ActiveBorderDelay Option values of ActiveBorder are compatible with old option values of ElectricBorder. Considering this, it might make sense either to revert renaming in this case or automatically convert these options using the release script.
Owner

Probably best to keep that for the next PR where I will be adding configurable actions for Active Corners.

Sounds good.
By the way, one of the improvement I will suggest is also to be able to tile windows to a quarter of the screen instead of half. Not sure this clashes with your ideas on active corners or if it is the same, but we can discuss after this PR is merged.

BTW, just noticed that due to Electric* -> Active* renaming some configuration options have been renamed:

  • ElectricBorder -> ActiveBorder
  • ElectricBorderDelay -> ActiveBorderDelay

Option values of ActiveBorder are compatible with old option values of ElectricBorder. Considering this, it might make sense either to revert renaming in this case or automatically convert these options using the release script.

There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*.
This way we don't need any conversion script and the update wil be painless for users.

> Probably best to keep that for the next PR where I will be adding configurable actions for Active Corners. Sounds good. By the way, one of the improvement I will suggest is also to be able to tile windows to a quarter of the screen instead of half. Not sure this clashes with your ideas on active corners or if it is the same, but we can discuss after this PR is merged. > BTW, just noticed that due to Electric* -> Active* renaming some configuration options have been renamed: > * ElectricBorder -> ActiveBorder > * ElectricBorderDelay -> ActiveBorderDelay > > Option values of ActiveBorder are compatible with old option values of ElectricBorder. Considering this, it might make sense either to revert renaming in this case or automatically convert these options using the release script. There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*. This way we don't need any conversion script and the update wil be painless for users.
Owner

Hi Philippe,

There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*.

after this is implemented, we can look at merging this PR. Looks like we may be able to add this to R14.1.1 instead of R14.2.0, that would be great.

EDIT: actually one more thing required before the fix. We need to edit the help docbook and add a short description of the new tiling functionality (I always forget about documentation...)

Hi Philippe, > There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*. after this is implemented, we can look at merging this PR. Looks like we may be able to add this to R14.1.1 instead of R14.2.0, that would be great. EDIT: actually one more thing required before the fix. We need to edit the help docbook and add a short description of the new tiling functionality (I always forget about documentation...)
Poster
Collaborator

There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*.

Done.

We need to edit the help docbook and add a short description of the new tiling functionality (I always forget about documentation...)

Done. I also added documentation on the new "Restore size of maximized/tiled windows when moving" option.

Please review and tell me if I can squash+merge it.

> There is an easy solution: when you read the options from theconfig file, you first check for the Active* options. If an Active* option is not found, try Electrical* and if even that one is not found, then use a default value. When you save, just save using the new name Active*. Done. > We need to edit the help docbook and add a short description of the new tiling functionality (I always forget about documentation...) Done. I also added documentation on the new "Restore size of maximized/tiled windows when moving" option. Please review and tell me if I can squash+merge it.
blu.256 modified the milestone from R14.2.0 release to R14.1.1 release 12 months ago
blu.256 requested review from MicheleC 12 months ago
MicheleC requested changes 11 months ago
MicheleC left a comment
Owner

A couple of minor adjustments needed, then we can squash, rebase and merge.

A couple of minor adjustments needed, then we can squash, rebase and merge.
/**
* @returns true if a maximized or tiled window should be reset to its original
* size when dragging it.
* @since R14.2.0
Owner

@since R14.1.1 if we decide to add to R14.1.x

@since R14.1.1 if we decide to add to R14.1.x
MicheleC marked this conversation as resolved
ActiveBottomLeft,
ActiveLeft,
ActiveTopLeft,
ELECTRIC_COUNT,
Owner

Suggest renaming ELECTRIC_COUNT into ACTIVE_COUNT

Suggest renaming ELECTRIC_COUNT into ACTIVE_COUNT
Poster
Collaborator

Yes, missed that, thanks!

Yes, missed that, thanks!
Owner

@SlavekB seems we don't brake any public API, so we can probably backport this to R14.1.1. What do you think?

@SlavekB seems we don't brake any public API, so we can probably backport this to R14.1.1. What do you think?
Owner

@SlavekB seems we don't brake any public API, so we can probably backport this to R14.1.1. What do you think?

Yes, definitely. This looks like a good new thing for R14.1.1.

> @SlavekB seems we don't brake any public API, so we can probably backport this to R14.1.1. What do you think? Yes, definitely. This looks like a good new thing for R14.1.1.
Owner

A couple of minor adjustments needed, then we can squash, rebase and merge.

@blu.256
not sure if you didn't get the previous notification or if you are just busy, so just a reminder :-)

> A couple of minor adjustments needed, then we can squash, rebase and merge. > @blu.256 not sure if you didn't get the previous notification or if you are just busy, so just a reminder :-)
Poster
Collaborator

Sorry, I forgot. I'll get to that in the afternoon.

Sorry, I forgot. I'll get to that in the afternoon.
blu.256 force-pushed feat/twin-electric-improvements from c704628e19 to 6fb6faffa3 11 months ago
MicheleC approved these changes 11 months ago
MicheleC left a comment
Owner

Looks good

Looks good
Owner

Ready for merging!
Don't forget to squash and rebase first.

Ready for merging! Don't forget to squash and rebase first.
blu.256 referenced this issue from a commit 11 months ago
blu.256 force-pushed feat/twin-electric-improvements from 6fb6faffa3 to 31335a04ed 11 months ago
Poster
Collaborator

Merging and backporting to r14.1.x

Merging and backporting to `r14.1.x`
blu.256 merged commit 31335a04ed into master 11 months ago
blu.256 deleted branch feat/twin-electric-improvements 11 months ago
blu.256 referenced this issue from a commit 11 months ago
Poster
Collaborator

✔️ Done

✔️ Done

Reviewers

MicheleC approved these changes 11 months ago
The pull request has been merged as 31335a04ed.
Sign in to join this conversation.
No reviewers
No Milestone
No Assignees
3 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: TDE/tdebase#331
Loading…
There is no content yet.