D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread David Faure
dfaure added a comment. In D14610#304619 , @rkflx wrote: > In D14610#303987 , @dfaure wrote: > > > Hmm, well, for IconApplet's use case > > > I'd say it would be nice to be consistent

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread Henrik Fehlauer
rkflx added a comment. In D14610#303987 , @dfaure wrote: > Hmm, well, for IconApplet's use case I'd say it would be nice to be consistent everywhere. > I could do it myself, faster than doing 10 reviews :-) No doubt about that,

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread Nathaniel Graham
ngraham added a comment. Late to the party here, but the read-only label needs to be selectable too. I submitted a patch for that: D14648 REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14610 To: shubham, rkflx, dfaure, ngraham Cc:

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread Nathaniel Graham
ngraham added a task: T9297: Polish file/folder properties dialog. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14610 To: shubham, rkflx, dfaure, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread Shubham
This revision was automatically updated to reflect the committed changes. Closed by commit R241:e2c56f6ddc51: Use KLineEdit for folder name if folder has write access, else use QLabel (authored by shubham). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. I disagree that it's more readable, but let's not nitpick :-) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14610 To: shubham, rkflx, dfaure, ngraham Cc:

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread David Faure
dfaure accepted this revision. dfaure added a comment. This revision is now accepted and ready to land. (oops, misclicked) REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14610 To: shubham, rkflx, dfaure, ngraham Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread Shubham
shubham updated this revision to Diff 39164. shubham added a comment. Add check for bFromTemplate simplify expression (!d->m_bFromTemplate && !itemList.supportsMoving()) to !(d->m_bFromTemplate || itemList.supportsMoving()) for better readability REPOSITORY R241 KIO CHANGES SINCE LAST

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-06 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Better, but you missed the `(!m_bFromTemplate &&` bit. Make sure test "Create New / Text File" in dolphin. REPOSITORY R241 KIO REVISION DETAIL

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread Shubham
shubham updated this revision to Diff 39163. shubham added a comment. Re factor Remove unnecessary indentations use only !itemList.supportsMoving() REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14610?vs=39112=39163 REVISION DETAIL

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread David Faure
dfaure added a comment. Hmm, well, for IconApplet's use case, if you never want a readonly line-edit then a major redesign is needed, switching from the lineedit to the qlabel inside of setFileNameReadOnly itself... (and using that in the code modified by this patch...). Or adding a ctor

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread David Faure
dfaure added a comment. `KFilePropsPlugin::setFileNameReadOnly` checks for m_bFromTemplate for the case where the properties dialog is used by the "Create New / ..." context menu action. In that case, the source file (the template from /usr) isn't movable, but we still want to let the

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread David Faure
dfaure requested changes to this revision. dfaure added a comment. This revision now requires changes to proceed. Please re-read your diff and, indeed, revert any unnecessary changes like indentation or no-op moving of code. INLINE COMMENTS > shubham wrote in kpropertiesdialog.cpp:988 >

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread Shubham
shubham added a comment. In D14610#303706 , @rkflx wrote: > In D14610#303537 , @rkflx wrote: > > > `KPropertiesDialog::setFileNameReadOnly` > > `m_bFromTemplate` > > > @shubham Any comments

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread Henrik Fehlauer
rkflx added a comment. In D14610#303537 , @rkflx wrote: > `KPropertiesDialog::setFileNameReadOnly` > `m_bFromTemplate` @shubham Any comments on that (see my questions to @dfaure above)? INLINE COMMENTS > kpropertiesdialog.cpp:988 > +

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-05 Thread Shubham
shubham updated this revision to Diff 39112. shubham added a comment. Re-use Qlabel REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14610?vs=39082=39112 REVISION DETAIL https://phabricator.kde.org/D14610 AFFECTED FILES src/widgets/kpropertiesdialog.cpp

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-04 Thread Henrik Fehlauer
rkflx added a comment. @shubham Thanks for helping out with T9297 ! > reuse the above code that creates a QLabel @dfaure Thanks for the review! Any advice on how to handle `KPropertiesDialog::setFileNameReadOnly`, which is used in Plasma's

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-04 Thread David Faure
dfaure requested changes to this revision. dfaure added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > kpropertiesdialog.cpp:994 > } > d->nameArea = lab; > } else { What about this code path? You broke it by removing the call to

D14610: Use KLineEdit for folder name if folder has write access, else use QLabel

2018-08-04 Thread Shubham
shubham retitled this revision from "Use KLineEdit if folder has write access, else use QLabel" to "Use KLineEdit for folder name if folder has write access, else use QLabel". REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14610 To: shubham, rkflx, dfaure, ngraham Cc: