> On Feb. 9, 2013, 6:49 p.m., Mark Gaiser wrote: > > Just wanted to add that it has a regression that slipped in the 4.10.0 > > release. https://bugs.kde.org/show_bug.cgi?id=312684 > > > > If you have only one virtual desktop the pager just shows an empty area. > > That is obviously wrong. It is actually very clearly visible in this - long > > - review: http://www.muktware.com/5194/kde-410-review-time-switch-kde. Just > > look at the first image, the gap between activity icon and dolphin is the > > pager. > > Luís Gabriel Lima wrote: > Actually it's not a regression. As you can see in the history of this > review, it was requested to hide the pager when there is only one virtual > desktop. Although I can see a bug in the current implementation, the pager > should be considerably small (practically hidden). Right now it's just making > the virtual desktop rect invisible and keeping the same size. > > Shaun Reich wrote: > Oh it most certainly is on at least some level. Yes, if the pager is > going to be hidden when one desktop is there, it shouldn't ghostily take up > any room. But I don't think hiding the widget just because there is one > desktop is a solution. In fact, I have seen users get confused around this, > because their system is set to 1 VD and they add a pager widget, and what > happens? Nothing. So they add it a few more times. Then they try adding it to > a different panel, it still isn't working. > > Turns out I had to tell them that they had to change the VD count to > 1, > and surprise surprise! there's 20 pager widgets all over the desktop. > > This is another case of someone trying to make something look clean and > minimalistic, but not thinking about the consequences when it comes to user > interaction. > > If you don't like how the pager widget looks with 1 desktop, then don't > ship it by default with 1 desktop, or don't ship the pager widget at all on a > default desktop (which honestly I don't think many users(aka average) would > use it in a default desktop). > > Turning the pager widget into a ghost widget is definitely not the answer > and we're going to get a lot of issues because of that, as I noted above, > because I witnessed them first hand... Making it invisible really just pushes > the confusion upward a bit more. > > Mark Gaiser wrote: > I understand your reasoning, but i don't think you should do this. In my > opinion a plasmoid - and certainly one that's on the panel - should never be > invisible but still take up room. If you do it you'd better show a big > warning notifying the user of that. Or you should just prohibit the plasmoid > if the user has just one virtual desktop. > > Another usecase that apparently hasn't been considered is my usecase > (obviously ;)). I have 3 "displays" attached. 2 monitors and one beamer. The > beamer obviously isn't on by default yet it is possible that windows open in > the beamer area. Thus i don't see those windows. If the pager would just show > itself i would instantly notice that some windows where opened on a display > that isn't turned on which would very easily allow me to drag those windows > back to one of the enabled displays. > > So yes, i think this is a regression. > > Also a point of critic about the code. > http://quickgit.kde.org/?p=kde-workspace.git&a=blob&h=228af8ec44f217553dec7ee7e10e1a0ffeda66dd&hb=1538ddeabae2488163f76a12080e6b152872f3a8&f=plasma%2Fdesktop%2Fapplets%2Fpager%2Fpager.cpp > > m_dummy = new Plasma::FrameSvg(this); > m_dummy->setImagePath("widgets/pager"); > Should be done through plasma QML. > > void Pager::updatePagerStyle() > Should all be done through QML. > > In short, all the painting - and how it should be painted - should be > done through QML. So if you want to set a different opacity then you simply > supply that value in the model and use it in QML. If you want to set the Z > level then you also provide that through the model and set it that way. > etcetera.. etcetera... > > If you need help with some QML stuff, you can ask me. > > Aaron J. Seigo wrote: > design discussion belongs on plasma-devel in a proper thread, not in the > comments section of this review. > > @Mark: the FrameSvg is not used for painting. the style information is > made available to the QML via a property, where it used by the QML to paint > and handle display. looking at the code, it seems that at least some of the > values would require extensions to the color scheme support we currently > offer to QML in Plasma Components. keep in mind that this is also a port from > C++ to C++/QML; the effort to get rid of all the C++ would hardly be > justified (in terms of effort/benefit) given that it would not provide any > benefits that i can see. certainly, if this was to be written from scratch > today we'd probably do it differently. we try to avoid wholesale rewrites, > however, when not necessary. > > Luís Gabriel Lima wrote: > @Mark: Can you create a thread on plasma-devel to discuss the hidden > problem? > > About you critics, Aaron already have answered them properly. The > FrameSvg is a dummy one (as its name suggests), and it's used to get the > margins of the framesvg. We need the margins to calculate the correct size > and positioning of the virtual desktop rects.
Aaron and Luis, Oke, will do. It will be roughly the same as i said above minus the code critics. - Mark ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/106112/#review27093 ----------------------------------------------------------- On Aug. 30, 2012, 2:30 a.m., Luís Gabriel Lima wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/106112/ > ----------------------------------------------------------- > > (Updated Aug. 30, 2012, 2:30 a.m.) > > > Review request for Plasma and Marco Martin. > > > Description > ------- > > This patch contains the QML port of the Pager plasmoid done during the GSoC > 2012. > > In this port basically I used QML to paint the Pager UI and deal with the > user interaction. > The geometry calculation of the desktop and window rectangles was kept in C++ > as well as other routines that needs to interact with classes like > KWindowSystem, QDbusConnection and so on. > This patch also introduces the PagerModel, a QAIM subclass that holds the > desktop/window geometries and is used by the QML part to fill the UI. > > > Diffs > ----- > > plasma/desktop/applets/pager/CMakeLists.txt 5d80514 > plasma/desktop/applets/pager/model.h PRE-CREATION > plasma/desktop/applets/pager/model.cpp PRE-CREATION > plasma/desktop/applets/pager/package/contents/ui/main.qml PRE-CREATION > plasma/desktop/applets/pager/package/contents/ui/utils.js PRE-CREATION > plasma/desktop/applets/pager/package/metadata.desktop PRE-CREATION > plasma/desktop/applets/pager/pager.h 6c7c045 > plasma/desktop/applets/pager/pager.cpp 74dc529 > > Diff: http://git.reviewboard.kde.org/r/106112/diff/ > > > Testing > ------- > > - Tested inside panels and floating on desktop, sizing works as expected > - Mouse interactions (move windows around, change desktop, etc) > > > Screenshots > ----------- > > > http://git.reviewboard.kde.org/r/106112/s/691/ > > http://git.reviewboard.kde.org/r/106112/s/692/ > > > Thanks, > > Luís Gabriel Lima > >
_______________________________________________ Plasma-devel mailing list Plasma-devel@kde.org https://mail.kde.org/mailman/listinfo/plasma-devel
