D12936: kcm_workspace is finished.

2018-05-16 Thread Furkan Tokac
furkantokac updated this revision to Diff 34343. furkantokac added a comment. Rewrite workspace KCM in QtQuick REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12936?vs=34332=34343 BRANCH bug393547-SingleDoubleClickFunc REVISION DETAIL

D12936: kcm_workspace is finished.

2018-05-16 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > kcm.cpp:133 > +// If yes, setNeedsSave(true). > +void KCMWorkspaceOptions::handleIfNeedSave() > +{ How about `handleNeedsSave()` instead? > furkantokac wrote in ToolTip.qml:2 > Yeap it's Roman's implementation used in "input" kcm. Shouldn't I

D12936: kcm_workspace is finished.

2018-05-16 Thread David Edmundson
davidedmundson added inline comments. INLINE COMMENTS > furkantokac wrote in metadata.desktop:105-107 > I'm removing. > Can you explain why ? Just want to learn. Those are special hints which are used only by plasma applets. This isn't a plasma applet. So they do nothing. REPOSITORY R119

D12936: kcm_workspace is finished.

2018-05-16 Thread Nathaniel Graham
ngraham added a comment. Also, this patch doesn't actually solve https://bugs.kde.org/show_bug.cgi?id=393547; it just ports the KCM to QML in preparation for fixing it. So please remove the `BUG 393547` keyword and let's only add it to the patch that actually re-adds the missing feature.

D12936: kcm_workspace is finished.

2018-05-16 Thread Furkan Tokac
furkantokac added a comment. In D12936#263939 , @ngraham wrote: > In D12936#263907 , @furkantokac wrote: > > > In D12936#263886 , @davidedmundson wrote:

D12936: kcm_workspace is finished.

2018-05-16 Thread Nathaniel Graham
ngraham added a comment. In D12936#263907 , @furkantokac wrote: > In D12936#263886 , @davidedmundson wrote: > > > Can you change the title to something more specific. > > > There will be

D12936: kcm_workspace is finished.

2018-05-16 Thread Furkan Tokac
furkantokac added a comment. In D12936#263894 , @ngraham wrote: > BTW, here are a couple of formatting rules you should follow: https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch Oh I know it but just forgot.

D12936: kcm_workspace is finished.

2018-05-16 Thread Nathaniel Graham
ngraham added inline comments. INLINE COMMENTS > ExclGroupBox.qml:1 > +/* > + * Copyright 2017 Roman Gilg Is this file actually used at all? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12936 To: furkantokac, romangg, ngraham Cc:

D12936: kcm_workspace is finished.

2018-05-16 Thread David Edmundson
davidedmundson added a comment. Can you change the title to something more specific. INLINE COMMENTS > ToolTip.qml:2 > +/* > + * Copyright 2017 Roman Gilg > + * is this copy pasted from somewhere? > main.qml:24 > + > +import org.kde.plasma.core 2.0 as PlasmaCore > +

D12936: kcm_workspace is finished.

2018-05-16 Thread Nathaniel Graham
ngraham added a comment. BTW, here are a couple of formatting rules you should follow: https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D12936 To: furkantokac, romangg, ngraham Cc:

D12936: kcm_workspace is finished.

2018-05-16 Thread Furkan Tokac
furkantokac created this revision. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. furkantokac requested review of this revision. REVISION SUMMARY kcm_workspaceoptions is changed as kcm_workspace. It is rewritten in QML, ConfigModule.