[Differential] [Requested Changes To] D3130: Improve Activity Pager Layout for horizontal and vertical cases

hein (Eike Hein) noreply at phabricator.kde.org
Mon Oct 24 16:58:36 UTC 2016


hein requested changes to this revision.
hein added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> main.qml:196
>  
> -            var columns = Math.floor(pagerModel.count / pagerModel.layoutRows);
> +            var rows=1;
> +

There's a lot of small coding style nits like no spaces around = here or no space after 'if' all throughout the patch - please clean that up a little bit.

> main.qml:199
> +            ///pagerLayout value supports 0-Default, 1-Horizontal, 2-Vertical
> +            if(isActivityPager && plasmoid.configuration.pagerLayout !== 0) {
> +                if (plasmoid.configuration.pagerLayout === 1){

Please add comments like /* Horizontal */ into the conditions for readability (I wish we could generate proper enums from config ...).

REPOSITORY
  rPLASMADESKTOP Plasma Desktop

REVISION DETAIL
  https://phabricator.kde.org/D3130

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: mvourlakos, mart, hein, #plasma
Cc: mvourlakos, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161024/f3e05bc6/attachment.html>


More information about the Plasma-devel mailing list