I've looked over your patch, and have a few notes:
* Please supply a unified diff (diff -u) and check that it isn't reversed (yours seems to be).
* Watch your indentation
* To me, names like "aotPixmap" and "m_aotted_pm" just don't click. Maybe call them something like toggleLayerUp and toggleLayerDown (toggleLayerUpPixmap, m_toggle_layer_up_pm). Yes, it's longer, but then I can read it without thinking so hard.
If you can fix these, I think we could commit it. In an ideal world, I'd be tempted to make it so that left click goes Top and right click goes Bottom (or some such).
Cheers,
- Simon
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
so i provide a unified patch against the 4646 version
i've tried to fix all the points in your notes
i coudn't use the existing methods FluxboxWindow::raiseLayer() and lowerLayer() because they navigate trough all the five layer states (ABOVE DOCK, ecc.), and i have to add my toUpperLayer() and toLowerLayer() that moves only between TOP,NORMAL and BOTTOM.
cheers
matteo
File Added: layersButtonedPatch.patch
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I suspect this to be some approach on the toggle raise/lower (within! a layer) request which will require some idea about the window stack (to checl for occlusions) for a proper implementation.
In that case, this patch is wrong (because it doesn't guarantees neither condition, there can be windows up in the atop layer and below in the normal layer)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
patch against the revision 4586
Logged In: YES
user_id=603593
Originator: NO
Hi,
I've looked over your patch, and have a few notes:
* Please supply a unified diff (diff -u) and check that it isn't reversed (yours seems to be).
* Watch your indentation
* To me, names like "aotPixmap" and "m_aotted_pm" just don't click. Maybe call them something like toggleLayerUp and toggleLayerDown (toggleLayerUpPixmap, m_toggle_layer_up_pm). Yes, it's longer, but then I can read it without thinking so hard.
If you can fix these, I think we could commit it. In an ideal world, I'd be tempted to make it so that left click goes Top and right click goes Bottom (or some such).
Cheers,
- Simon
patch against the revision 4646
Logged In: YES
user_id=697338
Originator: YES
so i provide a unified patch against the 4646 version
i've tried to fix all the points in your notes
i coudn't use the existing methods FluxboxWindow::raiseLayer() and lowerLayer() because they navigate trough all the five layer states (ABOVE DOCK, ecc.), and i have to add my toUpperLayer() and toLowerLayer() that moves only between TOP,NORMAL and BOTTOM.
cheers
matteo
File Added: layersButtonedPatch.patch
I suspect this to be some approach on the toggle raise/lower (within! a layer) request which will require some idea about the window stack (to checl for occlusions) for a proper implementation.
In that case, this patch is wrong (because it doesn't guarantees neither condition, there can be windows up in the atop layer and below in the normal layer)