rkflx added a comment.
Thanks for the help. Works great, just one inline question about an edge case (everything else LGTM). INLINE COMMENTS > kpropertiesdialog.cpp:853 > QGridLayout *grid = new QGridLayout(); // unknown rows > + d->m_grid = grid; > grid->setColumnStretch(0, 0); Just out of interest (don't change anything): I assume you don't fully transition everything to `d->m_grid` to keep the diff small, or because `grid` is more readable? > kpropertiesdialog.cpp:1211-1214 > + d->m_fileNameLabel = new QLabel(d->m_frame); > + > d->m_fileNameLabel->setTextInteractionFlags(Qt::TextSelectableByMouse | > Qt::TextSelectableByKeyboard); > + d->m_fileNameLabel->setText(d->oldName); // will get overwritten if > d->bMultiple > + d->m_grid->addWidget(d->m_fileNameLabel, 0, 2); Wouldn't this mean that calling `setFileNameReadOnly(true)` multiple times will also create multiple labels on top of each other, breaking idempotence and resulting in some kind of bold font effect? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D14662 To: dfaure, rkflx, shubham, ngraham Cc: bruns, michaelh, kde-frameworks-devel, ngraham
