-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
-----------------------------------------------------------



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48537>

    Is this needed? Adds a bit of overhead and you're not using it consistently 
everywhere anyway



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48538>

    Don't hardcode sizes, use units.gridUnit et al
    
    You also might want to make those properties readonly



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48539>

    Those don't seem t be used



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48540>

    Be careful with clipping, it's quite expensive



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48541>

    You don't need ; in qml definitions



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48542>

    Then use FrameSvg not FrameSvgItem



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48544>

    Is the lagging behind highlight really needed? :/



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48543>

    Margins only apply to corners where it's anchored to



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48545>

    Put id at the beginning of the item



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48546>

    Doesn't have a height



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48547>

    property variant is obsolete, use property var instead



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48548>

    spaces, foo * (1.0 / bar)



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48549>

    You don't need the {} when it's just one line



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48558>

    visible: false (space)



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48557>

    It's not anchored to the bottom, making the margin have no effect



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48550>

    Use units.iconSizes.xxx



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48556>

    Please check the order, it's all higgledy-piggledy



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48555>

    font.bold: true



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48551>

    Could this be done in a declarative, rather than imperative, way?



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48552>

    /**



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48554>

    This could be inlined in the label above
    text: {
      the code
    }



tabbox/qml/clients/scaling/contents/ui/main.qml
<https://git.reviewboard.kde.org/r/109832/#comment48553>

    i18n this?


- Kai Uwe Broulik


On Okt. 29, 2014, 11:38 vorm., Andre Heinecke wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/109832/
> -----------------------------------------------------------
> 
> (Updated Okt. 29, 2014, 11:38 vorm.)
> 
> 
> Review request for kwin, Plasma and Martin Gräßlin.
> 
> 
> Bugs: 292566
>     http://bugs.kde.org/show_bug.cgi?id=292566
> 
> 
> Repository: kde-workspace
> 
> 
> Description
> -------
> 
> I'm reopening this review request as I have updated this Window Switcher for 
> Plasma 5.1 and would like to get another review to check if there are any 
> suggestions or issues regarding the port to the new API.
> 
> Secondly I would like to ask again to have this Window Switcher Layout 
> included in the KWin repository. I would prefer it if users could obtain this 
> layout from their trusted distributors and did not have to rely on an 
> unverifyable third party download to obtain this plugin. 
> 
> As suggested in the original review I've put this up on kde-look and recieved 
> some positive feedback. But I really feel that it is rotting away there and 
> that KDE-Look is not a good place to distribute executable plugins.
> 
> IMHO the approach of this Window Switcher is different enough from the others 
> included in KWin to be a useful addition to the fold. Especially as this 
> behavior is already familiar to KDE users from some versions < 4.6
> 
> It should also be close enough to the other layouts like thumbnails to keep 
> maintenance very similar (I've mostly looked at the changes made to 
> thumbnails to adapt this for Plasma 5)
> 
> 
> Original description:
> 
> This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and 
> layout, it scales the thumbnails shown in the tabbox to avoid scrolling.
> There are also three different states in this layout depending on the size of 
> the scaled thumbnails to provide appropriate information even when there are 
> many opened windows.
> 
> States:
> 1. Thumbnails are larger then 200px: Show the Title and the Icon of the 
> Window directly below the thumbnail.
> 2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
> the icon but only the title of the currently selected window is shown.
> 3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
> shown and the title of the currently selected window (like big icons)
> If the Thumbnails would be smaller then 32px the tabbox starts to scroll in 
> the Icon Only state.
> 
> Size of the thumbnails depends on the screen size and the number of opened 
> windows.
> 
> 
> Diffs
> -----
> 
>   tabbox/qml/CMakeLists.txt fc55ab9 
>   tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
>   tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/109832/diff/
> 
> 
> Testing
> -------
> 
> Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.
> 
> 
> Thanks,
> 
> Andre Heinecke
> 
>

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

Reply via email to