D14662: KPropertiesDialog: switch to label in setFileNameReadOnly(true)

[Henrik Fehlauer]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180807/1ab61f0a/attachment.html>