Review Request: Manual panel hiding [WIP]
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3121/ --- Review request for Plasma. Summary --- Manual panel hiding patch (still in progress). Currently only horizontal panels are properly implemented, there is still a display bug whereby the plasma toolbox (cashew) ignores the contentsMargins set on the containment and thus gets covered by the hiding buttons. This addresses bug 158556. https://bugs.kde.org/show_bug.cgi?id=158556 Diffs - /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.h 1097398 /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.cpp 1097398 /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.h 1097398 /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.cpp 1097398 /trunk/KDE/kdebase/workspace/plasma/desktop/shell/scripting/panel.cpp 1097398 Diff: http://reviewboard.kde.org/r/3121/diff Testing --- Hiding the panels, changing states, changing positions and sizes. Thanks, Andrzej JR ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
On Thursday 04 March 2010 18:15:33 Aaron J. Seigo wrote: > > would you be able to upload a patch to reviewboard.kde.org so we can also > try it and comment on it directly? (if you don't feel it's ready for that > quite yet, that's cool; but when it is, please do so..) > > my thinking on the positioning is this: > > it probably shouldn't be in the containment itself. (though it's hard to > know exactly where it currently is being placed without seeing the patch :) > this is because the containment is completely free to do as it wants with > items inside of it ... including removing, hiding or even deleting them! The button is in the view, but I had set the contentsMargins in the containment, since editing those of the view did nothing whatsoever. I didn't particularly want to do this, since I have to assume that nothing else modifies the containment's margins, but it does work. Instead of passing the cashew out of the containment, the other possibility would be to reprogram it to respect the panel's content margins, i.e shift along of it's own accord. I have posted a first draft of the patch on the reviewboard. > the other problem i see there is fitts law: the buttons will not hit the > edges of the screen and that can be annoying and certainly make them harder > to use. The buttons appear to be small, however their actual size is larger, i.e. for the unhide button, not only the visible button, but the whole silver area is clickable. For the buttons on the panel the full 10px width x full panel height is clickable (i.e the same size of area as above). I'm just not sure how to set the button to (appearance wise) fill it's whole defined area. From the usability perspective I myself amn't experiencing any problems clicking on the button though. > to make it look good, we can perform a small trick: overlap the button with > the left/right margin area of the containment. in the case where the panel > hits the edge of screen, there is no overlap and the button and the > containment will meet with a nice perfectly square join. > > but in the case where the panel edge is floating, we may have a curved > edge. what to do? easy! take the panel-background svg and paint the > right/left border on top of the button with > QPainter::CompositionMode_Source. this will create the illusion that the > button is wrapped "around" the curvey edge of the panel and even with > translucency it will look perfect (the button parts that "stick out" in the > curved area will be visible, but the panel itself will be translucent). it > might sound trickier than it is, but it's really straight- forward, though > a bit hard to explain. i can also provide an example implementation once > you publish your patch. > > but this approach ensures that there is not interefering with the panel > containment itself. much simpler and won't end up with problems down the > road. > I'll try that then, although I'm not completely certain how you mean it yetl > > i think the only way to find out is to try it, test it and maybe even ship > it and get feedback. as you are the one implementing it (and you seem to be > really open to feedback, which is terrific), i'd say: make a decision, > implement it, and let's see how it goes from there. worst case we change > how it works later after we have some usage data :) > I'll just leave it with two buttons when centred then. I'll try monitor myself when using it to see how I use it, but I don't think this is a particularly major issue. -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Review Request: Manual panel hiding [WIP]
--- This is an automatically generated e-mail. To reply, visit: http://reviewboard.kde.org/r/3121/#review4378 --- Sorry, I've just noticed that there have been some changes to some of the affected files since my modifying them. I'll try and get an updated version ready soon. - Andrzej JR On 2010-03-04 19:19:53, Andrzej JR Hunt wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > http://reviewboard.kde.org/r/3121/ > --- > > (Updated 2010-03-04 19:19:53) > > > Review request for Plasma. > > > Summary > --- > > Manual panel hiding patch (still in progress). Currently only horizontal > panels are properly implemented, there is still a display bug whereby the > plasma toolbox (cashew) ignores the contentsMargins set on the containment > and thus gets covered by the hiding buttons. > > > This addresses bug 158556. > https://bugs.kde.org/show_bug.cgi?id=158556 > > > Diffs > - > > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.h 1097398 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelcontroller.cpp > 1097398 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.h 1097398 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/panelview.cpp 1097398 > /trunk/KDE/kdebase/workspace/plasma/desktop/shell/scripting/panel.cpp > 1097398 > > Diff: http://reviewboard.kde.org/r/3121/diff > > > Testing > --- > > Hiding the panels, changing states, changing positions and sizes. > > > Thanks, > > Andrzej JR > > ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
buttons as plasmoids so that each > user can decide where to place them (either inside the panel or on the > desktop). I'm concerned about the buttons taking the screen or panel edges, > and/or displacing or overlapping the panel contents. If this is the > behavior of this feature, I'd certainly ask to have an option to disable it > completely. Don't worry: the feature is completely optional: it is simply an additional panel visiblity mode, i.e. instead of selecting "Always Visible", "Windows Can Cover" and the like, you select "manually hideable", i.e. the feature only appears if the user specifically selects it, and the default panel visiblity modes, and their behaviousr, are still the same as ever. -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
> apart that ToolButton should be used instead, don't really like a > fuction that does enabling -and- repositioning in one. positioning should > be done by a layout, really > I now have a layout, however I'm having to call update() on it everytime I change the button's size since it doesn't realise this itself, which seems a bit strange to me. On the other hand no position calculations are required, making life simpler. > i'd also position the buttons so they overlap the border of the svg. > e.g. for the right-hand button in a horizontal panel, something like: > qreal left, top, right, bottom; > containment()->getContentsMargins(&left, &top, &right, &bottom) > setContentsMargins(0, 0, m_hideButton.width() - int(right), 0); > m_hideButton->move(0, geometry().right() - m_hideButton.width()); > > (obviously would need to be generalized for vertical panels and > top/left/right/bottom buttons) I've now implemented the buttons as part of a layout (which is part of the view), so the positioning isn't required, however what I have noticed is that for setContentsMargins(left, top...), when used on the view, only the top and bottom values have any influence, with the left and right values not doing anything. However the margins on the view don't affect the containment (i.e. only these hiding buttons are affected, the containment behaves in exactly the same way), and the problem is I need the containment to know that it isn't allowed to put anything below where the hiding buttons are, which I only seem to be able to do by setting its margins, and even then, while the applets on the panel move correctly, the cashew stays at the end of the panel, leaving a gap between it and the first applet, instead of the gap being beneath the hide button. (Without the cashew everything works fine.) One solution I'm thinking of now is adding a special method to the panel containment such as requestEmptySpace(int left, int top, ...), which in the containment creates empty spacer objects, similar to the cashew which is also permanent (i.e. separate from the applets), giving the required space without editing the contentsMargins. (A better method might be requestEndSpace(int leftTop, int rightBottom), since we only need the spaces at the ends.) This also means the cashew doesn't have to be modified to move with the contents margins since the spacer objects already shift it along by the required amount. -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Layout and spacing
Yes, there is a simpler solution: assuming you are using a layout supporting insertion of stretches you can use: layout.addStretch(1); after adding the buttons and before adding the LineEdit onto the layout, which pushes the linedit to the other side of the layout, by inserting a spacer that resizes to be as wide as possible. On Saturday 06 March 2010 18:31:04 Cédric Bellegarde wrote: > Hello, > > on the attached screenshot, you will see an horizontal layout with buttons > and a LineEdit. > > What i want is buttons on the left and LineEdit on the right. > > The only solution i see is calculate layout size, and items size and do: > layout.setItemSpacing (LastButtonIndex, (layoutWidth - ItemsWidth)) > > Am i right, is there a simpler solution? > > Thanks. > > Cédric -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
On Saturday 06 March 2010 18:52:52 Aaron J. Seigo wrote: > i don't think this will work very well because we will want the button to > overlap with the View of the containment (for visual reasons). layouts > prevent that kind of overlapping (that's mostly the point of them :) and > the position calculations in this case are not hard to do. I'm not sure if I'm understanding you corectly: are you suggesting that the buttons are drawn partly outside the area where the panel (i.e. containment) is drawn (in other words making the panel containment smaller than the view), and then drawing the additional background to make this fit in? The one problem I can see there though are that I have to disable the rounded corners of the panel (I'll have a look to see how they are currently determined / set.). (The layout shouldn't make much of a difference in this method though, since at the moment it does exactly what my positioning calculations were doing.) -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
On Sunday 07 March 2010 14:43:49 you wrote: > On Sunday 07 March 2010, Andrzej JR Hunt wrote: > > On Saturday 06 March 2010 18:52:52 Aaron J. Seigo wrote: > > > i don't think this will work very well because we will want the button > > > to overlap with the View of the containment (for visual reasons). > > > layouts prevent that kind of overlapping (that's mostly the point of > > > them :) and the position calculations in this case are not hard to do. > > > > I'm not sure if I'm understanding you corectly: are you suggesting that > > the buttons are drawn partly outside the area where the panel (i.e. > > containment) is drawn (in other words making the panel containment > > smaller than the view), and then drawing the additional background to > > make this > > there is the misunderstanding: right now the panel window -is- a view: it's > not something set on stone: the view itself can be layouted in the form > button-view-button, or more probably, the anel still being a view but with > its viewport shrinked to make room for the buttons I'm still slightly confused... Just to make sure I understand everything right: >From what I have seen the panel itself is a containment, this also does the painting of the panel, places the cashew on it, has the applet placed on it. The panelView is responsible for hiding/showing this etc: the panel is inside the view, and until now completely fills it. - Up until now I have only really added the buttons on top of the view. and have tried to get the panel (containment) to leave extra blank spaces so that the buttons have space (using setContentMargins()). This method however isn't particularly good because it places assumptions on the containment about its margins which shouldn't be made as not to break anything. (This method also doesn't work because of the cashew not moving...) - The suggested method is to make the panel containment smaller than the view, then adding the buttons in the empty space at either end of the view, and painting the background to make it one with the containment? (This would require rewriting all of the view's calls to resize the containment to leave the required space, and possibly to position it, since the containment isn't currently affected by the contents margins of the view, and the viewport margins of the view only affect the visible area, but not the actual size, of the panel containment. Alternatively the panel containment has to be made part of a layout inside the view.) - Putting the buttons as part of the containment -- but only the panel containment class, not the overall plasma:containment class -- (but leaving the hiding functions in the view) still isn't OK? (I was just thinking about this: the cashew and panel painting are all done by the containment: since the buttons themselves are a display/control issue, as opposed to the hiding which is a view issue, should they not also be dealt with by the containment?) > > fit in? The one problem I can see there though are that I have to disable > > the rounded corners of the panel (I'll have a look to see how they are > > currently determined / set.). (The layout shouldn't make much of a > > difference in this method though, since at the moment it does exactly > > what my positioning calculations were doing.) > > you don't have to disable them, just paint -over- them, on compositing but > replacing wat that was already painted That isn't too difficult in itself, but if the cashew is showing I need to paint more of it on, since when the panel end is curved, the curve cuts off part of the cashew (it gets placed at the very end of the panel), but the panel now has the curve further away, so we need to fill in what wasn't painted with the extra parts of the cashew. Or does the compositing keep the border programaticallt separate from the contents of the panel (when drawn), so that I can somehow easily remove it? -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
Re: Manual Hiding of Plasma Panel (desktop shell)
> > - The suggested method is to make the panel containment smaller than the > > view, then adding the buttons in the empty space at either end of the > > view, and painting the background to make it one with the containment? > > (This would require rewriting all of the view's calls to resize the > > containment to leave the required space, and possibly to position it, > > since the containment isn't currently affected by the contents margins of > > the view, and the viewport margins of the view only affect the visible > > area, but not the actual size, of the panel containment. Alternatively > > the panel containment has to be made part of a layout inside the view.) > > yes, PanelView could become a QWidget with a Plasma::View inside it and the > buttons on either side. > > but taking the contentsMargins into consideration when sizing the > Containment is probably easier and more straightforward. Ok, I think I'm finally on the right track now. I've just gotten the latest svn copy, and to test it out have tried setting the contents margins there (with nothing else enabled from the manual hiding code, i.e. a new clean checout): setting the contents margins still doesn't affect the containments size, so I'll probably have to place the current view in a QWidget (or should I look into making Plasma::View take its contents margins into account when resizing the containment it shows, i.e. is this actually a plasma bug that the contentsmargins don't do anything? Doing this would massively simplify the modifications to panelview...). I'm not really sure how to set about doing the QWidget version though in the correct way: panelview is used in quite a few ther classes, so if I were to change it into a QWidget I'd need to make quite a few changes to other surrounding files. Would it be correct to simply restructure the current PanelView, by just taking everything view specific into a separate class (PanelInternalView), which is then used (inside the QWidget) as part of the new PanelView, making sure all resizing calls etc. go to the new QWidget based PanelView, as to ensure good behaviour? Thanks for the help so far as well :). -- Andrzej JR Hunt -- andrzej (at) ahunt.org ___ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
