Differential D12321 Diff 32500 src/filewidgets/kdiroperator.cpp

Changeset View

Standalone View

View Options

src/filewidgets/kdiroperator.cpp

Show First 20 Lines • Show All 223 Lines • ▼ Show 20 Line(s)172public:
224 void _k_assureVisibleSelection(); 224​ void _k_assureVisibleSelection();
225 void _k_synchronizeSortingState(int, Qt::SortOrder); 225​ void _k_synchronizeSortingState(int, Qt::SortOrder);
226 void _k_slotChangeDecorationPosition(); 226​ void _k_slotChangeDecorationPosition();
227 void _k_slotExpandToUrl(const QModelIndex &); 227​ void _k_slotExpandToUrl(const QModelIndex &);
228 void _k_slotItemsChanged(); 228​ void _k_slotItemsChanged();
229 void _k_slotDirectoryCreated(const QUrl &); 229​ void _k_slotDirectoryCreated(const QUrl &);
230 230​
231 void updateListViewGrid(); 231​ void updateListViewGrid();
232 int iconSizeForViewType(QAbstractItemView *itemView) const; 232​ int iconSizeForViewType(QAbstractItemView *itemView) const;
rkflxUnsubmitted
Done

I'd name this updatePreviewActionState to keep the terminology somewhat consistent (otherwise the code is a bit hard to follow, at least I felt this way when reviewing the patch).

rkflx: I'd name this `updatePreviewActionState` to keep the terminology somewhat consistent (otherwise…
233 233​
234 // private members 234​ // private members
235 KDirOperator *parent; 235​ KDirOperator *parent;
236 QStack<QUrl *> backStack; ///< Contains all URLs you can reach with the back button. 236​ QStack<QUrl *> backStack; ///< Contains all URLs you can reach with the back button.
237 QStack<QUrl *> forwardStack; ///< Contains all URLs you can reach with the forward button. 237​ QStack<QUrl *> forwardStack; ///< Contains all URLs you can reach with the forward button.
238 238​
239 QModelIndex lastHoveredIndex; 239​ QModelIndex lastHoveredIndex;
240 240​
Show All 36 Lines
277 277​
278 KNewFileMenu *newFileMenu; 278​ KNewFileMenu *newFileMenu;
279 279​
280 KConfigGroup *configGroup; 280​ KConfigGroup *configGroup;
281 281​
282 KFilePreviewGenerator *previewGenerator; 282​ KFilePreviewGenerator *previewGenerator;
283 283​
284 bool showPreviews; 284​ bool showPreviews;
285 int iconsZoom; 285​ int iconsZoom;
rkflxUnsubmitted
Done

calledFromUpdatePreviewActionState

rkflx: `calledFromUpdatePreviewActionState`
286 286​
rkflxUnsubmitted
Done

showPreviewsConfigEntry

rkflx: `showPreviewsConfigEntry`
287 bool isSaving; 287​ bool isSaving;
288 288​
289 KActionMenu *decorationMenu; 289​ KActionMenu *decorationMenu;
290 KToggleAction *leftAction; 290​ KToggleAction *leftAction;
291 QList<QUrl> itemsToBeSetAsCurrent; 291​ QList<QUrl> itemsToBeSetAsCurrent;
292 bool shouldFetchForItems; 292​ bool shouldFetchForItems;
293 InlinePreviewState inlinePreviewState; 293​ InlinePreviewState inlinePreviewState;
294 QStringList supportedSchemes; 294​ QStringList supportedSchemes;
▲ Show 20 Lines • Show All 1333 Lines • ▼ Show 20 Line(s)1538{
1628 const bool previewShown = d->inlinePreviewState == Private::NotForced ? d->showPreviews : previewForcedToTrue; 1628​ const bool previewShown = d->inlinePreviewState == Private::NotForced ? d->showPreviews : previewForcedToTrue;
1629 d->previewGenerator = new KFilePreviewGenerator(d->itemView); 1629​ d->previewGenerator = new KFilePreviewGenerator(d->itemView);
1630 const int maxSize = KIconLoader::SizeEnormous - KIconLoader::SizeSmall; 1630​ const int maxSize = KIconLoader::SizeEnormous - KIconLoader::SizeSmall;
1631 const int val = (maxSize * d->iconsZoom / 100) + KIconLoader::SizeSmall; 1631​ const int val = (maxSize * d->iconsZoom / 100) + KIconLoader::SizeSmall;
1632 d->itemView->setIconSize(previewForcedToTrue ? QSize(KIconLoader::SizeHuge, KIconLoader::SizeHuge) : QSize(val, val)); 1632​ d->itemView->setIconSize(previewForcedToTrue ? QSize(KIconLoader::SizeHuge, KIconLoader::SizeHuge) : QSize(val, val));
1633 d->previewGenerator->setPreviewShown(previewShown); 1633​ d->previewGenerator->setPreviewShown(previewShown);
1634 d->actionCollection->action(QStringLiteral("inline preview"))->setChecked(previewShown); 1634​ d->actionCollection->action(QStringLiteral("inline preview"))->setChecked(previewShown);
1635 1635​
1636​ // set grid and previews
1637​ d->updateListViewGrid();
rkflxUnsubmitted
Done

Unrelated?

rkflx: Unrelated?
1638​
1636 // ensure we change everything needed 1639​ // ensure we change everything needed
1637 d->_k_slotChangeDecorationPosition(); 1640​ d->_k_slotChangeDecorationPosition();
1638 1641​
1639 emit viewChanged(view); 1642​ emit viewChanged(view);
1640 1643​
1641 const int zoom = previewForcedToTrue ? (KIconLoader::SizeHuge - KIconLoader::SizeSmall + 1) * 100 / maxSize : d->iconSizeForViewType(view); 1644​ const int zoom = previewForcedToTrue ? (KIconLoader::SizeHuge - KIconLoader::SizeSmall + 1) * 100 / maxSize : d->iconSizeForViewType(view);
1642 1645​
1643 // this will make d->iconsZoom be updated, since setIconsZoom slot will be called 1646​ // this will make d->iconsZoom be updated, since setIconsZoom slot will be called
▲ Show 20 Lines • Show All 466 Lines • ▼ Show 20 Line(s)2112 } else if (viewStyle == QLatin1String("DetailTree")) {
2110 d->defaultView |= KFile::DetailTree; 2113​ d->defaultView |= KFile::DetailTree;
2111 } else { 2114​ } else {
2112 d->defaultView |= KFile::Simple; 2115​ d->defaultView |= KFile::Simple;
2113 } 2116​ }
2114 //if (configGroup.readEntry(QLatin1String("Separate Directories"), 2117​ //if (configGroup.readEntry(QLatin1String("Separate Directories"),
2115 // DefaultMixDirsAndFiles)) { 2118​ // DefaultMixDirsAndFiles)) {
2116 // d->defaultView |= KFile::SeparateDirs; 2119​ // d->defaultView |= KFile::SeparateDirs;
2117 //} 2120​ //}
2118 if (configGroup.readEntry(QStringLiteral("Show Preview"), false)) { 2121​ if (configGroup.readEntry(QStringLiteral("Show Preview"), false)) {
rkflxUnsubmitted
Done

That's a totally unrelated change (and a change which I would object to). Show Preview is about the preview sidebar you can toggle with F11.

rkflx: That's a totally unrelated change (and a change which I would object to). `Show Preview` is…
2119 d->defaultView |= KFile::PreviewContents; 2122​ d->defaultView |= KFile::PreviewContents;
2120 } 2123​ }
2121 2124​
2122 d->previewWidth = configGroup.readEntry(QStringLiteral("Preview Width"), 100); 2125​ d->previewWidth = configGroup.readEntry(QStringLiteral("Preview Width"), 100);
2123 2126​
2124 if (configGroup.readEntry(QStringLiteral("Show hidden files"), 2127​ if (configGroup.readEntry(QStringLiteral("Show hidden files"),
2125 DefaultShowHidden)) { 2128​ DefaultShowHidden)) {
2126 d->actionCollection->action(QStringLiteral("show hidden"))->setChecked(true); 2129​ d->actionCollection->action(QStringLiteral("show hidden"))->setChecked(true);
Show All 17 Lines2146 } else if (sortBy == QLatin1String("Type")) {
2144 sorting |= QDir::Type; 2147​ sorting |= QDir::Type;
2145 } 2148​ }
2146 if (configGroup.readEntry(QStringLiteral("Sort reversed"), DefaultSortReversed)) { 2149​ if (configGroup.readEntry(QStringLiteral("Sort reversed"), DefaultSortReversed)) {
2147 sorting |= QDir::Reversed; 2150​ sorting |= QDir::Reversed;
2148 } 2151​ }
2149 d->updateSorting(sorting); 2152​ d->updateSorting(sorting);
2150 2153​
2151 if (d->inlinePreviewState == Private::NotForced) { 2154​ if (d->inlinePreviewState == Private::NotForced) {
2152 d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), false); 2155​ d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), false);
ngrahamUnsubmitted
Done

Unintentional changes from D12328

ngraham: Unintentional changes from D12328
2153 } 2156​ }
rkflxUnsubmitted
Done

Don't remove that, otherwise your reviewer will be totally confused and wonder why your patch is suddenly totally broken! :D

(I suspect you already removed what Nate wanted in an earlier revision, but the rest should be kept, of course…)

rkflx: Don't remove that, otherwise your reviewer will be totally confused and wonder why your patch…
2154 QStyleOptionViewItem::Position pos = (QStyleOptionViewItem::Position) configGroup.readEntry(QStringLiteral("Decoration position"), (int) QStyleOptionViewItem::Left); 2157​ QStyleOptionViewItem::Position pos = (QStyleOptionViewItem::Position) configGroup.readEntry(QStringLiteral("Decoration position"), (int) QStyleOptionViewItem::Left);
2155 setDecorationPosition(pos); 2158​ setDecorationPosition(pos);
2156} 2159​}
2157 2160​
2158void KDirOperator::writeConfig(KConfigGroup &configGroup) 2161​void KDirOperator::writeConfig(KConfigGroup &configGroup)
2159{ 2162​{
2160 QString sortBy = QStringLiteral("Name"); 2163​ QString sortBy = QStringLiteral("Name");
2161 if (KFile::isSortBySize(d->sorting)) { 2164​ if (KFile::isSortBySize(d->sorting)) {
▲ Show 20 Lines • Show All 46 Lines • ▼ Show 20 Line(s)
2208 } else if (KFile::isTreeView(fv)) { 2211​ } else if (KFile::isTreeView(fv)) {
2209 style = QStringLiteral("Tree"); 2212​ style = QStringLiteral("Tree");
2210 } else if (KFile::isDetailTreeView(fv)) { 2213​ } else if (KFile::isDetailTreeView(fv)) {
2211 style = QStringLiteral("DetailTree"); 2214​ style = QStringLiteral("DetailTree");
2212 } 2215​ }
2213 configGroup.writeEntry(QStringLiteral("View Style"), style); 2216​ configGroup.writeEntry(QStringLiteral("View Style"), style);
2214 2217​
2215 if (d->inlinePreviewState == Private::NotForced) { 2218​ if (d->inlinePreviewState == Private::NotForced) {
2216 configGroup.writeEntry(QStringLiteral("Previews"), d->showPreviews); 2219​ configGroup.writeEntry(QStringLiteral("Previews"), d->showPreviews);
ngrahamUnsubmitted
Done

Unintentional changes from D12328

ngraham: Unintentional changes from D12328
2217 if (qobject_cast<QListView *>(d->itemView)) { 2220​ if (qobject_cast<QListView *>(d->itemView)) {
2218 configGroup.writeEntry(QStringLiteral("listViewIconSize"), d->iconsZoom); 2221​ configGroup.writeEntry(QStringLiteral("listViewIconSize"), d->iconsZoom);
2219 } else { 2222​ } else {
2220 configGroup.writeEntry(QStringLiteral("detailedViewIconSize"), d->iconsZoom); 2223​ configGroup.writeEntry(QStringLiteral("detailedViewIconSize"), d->iconsZoom);
2221 } 2224​ }
2222 } 2225​ }
2223 2226​
2224 configGroup.writeEntry(QStringLiteral("Decoration position"), (int) d->decorationPosition); 2227​ configGroup.writeEntry(QStringLiteral("Decoration position"), (int) d->decorationPosition);
▲ Show 20 Lines • Show All 337 Lines • ▼ Show 20 Line(s)
2562} 2565​}
2563 2566​
2564void KDirOperator::Private::updateListViewGrid() 2567​void KDirOperator::Private::updateListViewGrid()
2565{ 2568​{
2566 if (!itemView) { 2569​ if (!itemView) {
2567 return; 2570​ return;
2568 } 2571​ }
2569 2572​
2573​ const QFontMetrics metrics(itemView->viewport()->font());
2574​
2575​ // hide icon previews when they are too small
rkflxUnsubmitted
Done

Unrelated change.

rkflx: Unrelated change.
2576​ if (showPreviews) {
rkflxUnsubmitted
Done

Please drop the is in the variable name, as this would only be used for the corresponding getter function normally.

rkflx: Please drop the `is` in the variable name, as this would only be used for the corresponding…
2577​ previewGenerator->setPreviewShown(itemView->iconSize().height() > metrics.height() * 2);
2578​ }
elvisangelaccioUnsubmitted
Done

qobject_cast

elvisangelaccio: `qobject_cast`
2579​
2570 QListView *view = qobject_cast<QListView *>(itemView); 2580​ QListView *view = qobject_cast<QListView *>(itemView);
rkflxUnsubmitted
Done

Could you explain why you need to change showPreviewsEnabledBeforeZoom here? As far as I can see this variable just caches the config value, and thus should only be set in _k_toggleInlinePreviews?

Do you have an example where something would break if we would remove this code block?

rkflx: Could you explain why you need to change `showPreviewsEnabledBeforeZoom` here? As far as I can…
anemethAuthorUnsubmitted
Not Done

Without this the previews are always disabled when opening the dialog. This was added to enable it and then disable it right after if the read config was disabled in the k_toggleInlinePreviews function.
This will be fixed with D12328

anemeth: Without this the previews are always disabled when opening the dialog. This was added to enable…
2571 2581​
2572 if (!view) { 2582​ if (!view) {
2573 return; 2583​ return;
2574 } 2584​ }
elvisangelaccioUnsubmitted
Not Done

Maybe we can also add an hint about how to actually show the previews? (i.e. the fact that the user needs to increase the icon size).

elvisangelaccio: Maybe we can also add an hint about how to actually show the previews? (i.e. the fact that the…
anemethAuthorUnsubmitted
Not Done

Do you have a suggestion?
I think it is self explanatory.
@ngraham what do you think?

anemeth: Do you have a suggestion? I think it is self explanatory. @ngraham what do you think?
ngrahamUnsubmitted
Not Done

@anemeth I'm inclined to agree, though there's always room for improvement, and more clarity is usually better. How about this for the tooltip text?

"Automatically disabled for small icons; increase icon size to see previews"

ngraham: @anemeth I'm inclined to agree, though there's always room for improvement, and more clarity is…
2575 2585​
2576 const bool leftChecked = actionCollection->action(QStringLiteral("decorationAtLeft"))->isChecked(); 2586​ const bool leftChecked = actionCollection->action(QStringLiteral("decorationAtLeft"))->isChecked();
2577 2587​
2578 if (leftChecked) { 2588​ if (leftChecked) {
rkflxUnsubmitted
Done

Wording: "for preview" → "for showing previews"

(Or just use @ngraham's wording instead of inventing your own, because as a native speaker he often gets it quite right: "Previews are automatically disabled for icons smaller than {threshold]", or simply "Previews are automatically disabled for small icon sizes.".)

rkflx: Wording: "for preview" → "for showing previews" (Or just use @ngraham's wording instead of…
2579 view->setGridSize(QSize()); 2589​ view->setGridSize(QSize());
2580 KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate()); 2590​ KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate());
2581 if (delegate) { 2591​ if (delegate) {
2582 delegate->setMaximumSize(QSize()); 2592​ delegate->setMaximumSize(QSize());
2583 } 2593​ }
2584 } else { 2594​ } else {
2585 const QFontMetrics metrics(itemView->viewport()->font()); ​
rkflxUnsubmitted
Done

Unrelated change.

rkflx: Unrelated change.
2586 int size = itemView->iconSize().height() + metrics.height() * 2; 2595​ int size = itemView->iconSize().height() + metrics.height() * 2;
rkflxUnsubmitted
Done

You still got an unrelated whitespace change here (check on Phabricator with Revision Contents → History → Whitespace Changes → Show All).

rkflx: You still got an unrelated whitespace change here (check on Phabricator with {nav Revision…
2587 // some heuristics for good looking. let's guess width = height * (3 / 2) is nice 2596​ // some heuristics for good looking. let's guess width = height * (3 / 2) is nice
2588 view->setGridSize(QSize(size * (3.0 / 2.0), size + metrics.height())); 2597​ view->setGridSize(QSize(size * (3.0 / 2.0), size + metrics.height()));
2589 KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate()); 2598​ KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate());
2590 if (delegate) { 2599​ if (delegate) {
2591 delegate->setMaximumSize(QSize(size * (3.0 / 2.0), size + metrics.height())); 2600​ delegate->setMaximumSize(QSize(size * (3.0 / 2.0), size + metrics.height()));
2592 } 2601​ }
2593 } 2602​ }
2594} 2603​}
▲ Show 20 Lines • Show All 74 Lines • Show Last 20 Lines