Differential D12321 Diff 33331 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​ void updatePreviewActionState();
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…
232 int iconSizeForViewType(QAbstractItemView *itemView) const; 233​ int iconSizeForViewType(QAbstractItemView *itemView) const;
233 234​
234 // private members 235​ // private members
235 KDirOperator *parent; 236​ KDirOperator *parent;
236 QStack<QUrl *> backStack; ///< Contains all URLs you can reach with the back button. 237​ 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. 238​ QStack<QUrl *> forwardStack; ///< Contains all URLs you can reach with the forward button.
238 239​
239 QModelIndex lastHoveredIndex; 240​ QModelIndex lastHoveredIndex;
Show All 37 Lines
277 278​
278 KNewFileMenu *newFileMenu; 279​ KNewFileMenu *newFileMenu;
279 280​
280 KConfigGroup *configGroup; 281​ KConfigGroup *configGroup;
281 282​
282 KFilePreviewGenerator *previewGenerator; 283​ KFilePreviewGenerator *previewGenerator;
283 284​
284 bool showPreviews; 285​ bool showPreviews;
286​ bool calledFromUpdatePreviewActionState;
rkflxUnsubmitted
Done

calledFromUpdatePreviewActionState

rkflx: `calledFromUpdatePreviewActionState`
287​ bool showPreviewsConfigEntry;
rkflxUnsubmitted
Done

showPreviewsConfigEntry

rkflx: `showPreviewsConfigEntry`
288​
285 int iconsZoom; 289​ int iconsZoom;
286 290​
287 bool isSaving; 291​ bool isSaving;
288 292​
289 KActionMenu *decorationMenu; 293​ KActionMenu *decorationMenu;
290 KToggleAction *leftAction; 294​ KToggleAction *leftAction;
291 QList<QUrl> itemsToBeSetAsCurrent; 295​ QList<QUrl> itemsToBeSetAsCurrent;
292 bool shouldFetchForItems; 296​ bool shouldFetchForItems;
Show All 18 Lines301KDirOperator::Private::Private(KDirOperator *_parent) :
311 progressDelayTimer(nullptr), 315​ progressDelayTimer(nullptr),
312 dropOptions(0), 316​ dropOptions(0),
313 actionMenu(nullptr), 317​ actionMenu(nullptr),
314 actionCollection(nullptr), 318​ actionCollection(nullptr),
315 newFileMenu(nullptr), 319​ newFileMenu(nullptr),
316 configGroup(nullptr), 320​ configGroup(nullptr),
317 previewGenerator(nullptr), 321​ previewGenerator(nullptr),
318 showPreviews(false), 322​ showPreviews(false),
323​ calledFromUpdatePreviewActionState(false),
324​ showPreviewsConfigEntry(false),
319 iconsZoom(0), 325​ iconsZoom(0),
320 isSaving(false), 326​ isSaving(false),
321 decorationMenu(nullptr), 327​ decorationMenu(nullptr),
322 leftAction(nullptr), 328​ leftAction(nullptr),
323 shouldFetchForItems(false), 329​ shouldFetchForItems(false),
324 inlinePreviewState(NotForced) 330​ inlinePreviewState(NotForced)
325{ 331​{
326} 332​}
▲ Show 20 Lines • Show All 322 Lines • ▼ Show 20 Line(s)
649void KDirOperator::Private::_k_toggleInlinePreviews(bool show) 655​void KDirOperator::Private::_k_toggleInlinePreviews(bool show)
650{ 656​{
651 if (showPreviews == show) { 657​ if (showPreviews == show) {
652 return; 658​ return;
653 } 659​ }
654 660​
655 showPreviews = show; 661​ showPreviews = show;
656 662​
663​ if (!calledFromUpdatePreviewActionState) {
664​ showPreviewsConfigEntry = show;
665​ }
666​
657 if (!previewGenerator) { 667​ if (!previewGenerator) {
658 return; 668​ return;
659 } 669​ }
660 670​
661 previewGenerator->setPreviewShown(show); 671​ previewGenerator->setPreviewShown(show);
662} 672​}
663 673​
664void KDirOperator::Private::_k_slotOpenFileManager() 674​void KDirOperator::Private::_k_slotOpenFileManager()
▲ Show 20 Lines • Show All 963 Lines • ▼ Show 20 Line(s)1548{
1628 const bool previewShown = d->inlinePreviewState == Private::NotForced ? d->showPreviews : previewForcedToTrue; 1638​ const bool previewShown = d->inlinePreviewState == Private::NotForced ? d->showPreviews : previewForcedToTrue;
1629 d->previewGenerator = new KFilePreviewGenerator(d->itemView); 1639​ d->previewGenerator = new KFilePreviewGenerator(d->itemView);
1630 const int maxSize = KIconLoader::SizeEnormous - KIconLoader::SizeSmall; 1640​ const int maxSize = KIconLoader::SizeEnormous - KIconLoader::SizeSmall;
1631 const int val = (maxSize * d->iconsZoom / 100) + KIconLoader::SizeSmall; 1641​ const int val = (maxSize * d->iconsZoom / 100) + KIconLoader::SizeSmall;
1632 d->itemView->setIconSize(previewForcedToTrue ? QSize(KIconLoader::SizeHuge, KIconLoader::SizeHuge) : QSize(val, val)); 1642​ d->itemView->setIconSize(previewForcedToTrue ? QSize(KIconLoader::SizeHuge, KIconLoader::SizeHuge) : QSize(val, val));
1633 d->previewGenerator->setPreviewShown(previewShown); 1643​ d->previewGenerator->setPreviewShown(previewShown);
1634 d->actionCollection->action(QStringLiteral("inline preview"))->setChecked(previewShown); 1644​ d->actionCollection->action(QStringLiteral("inline preview"))->setChecked(previewShown);
1635 1645​
1636 // ensure we change everything needed 1646​ // ensure we change everything needed
1637 d->_k_slotChangeDecorationPosition(); 1647​ d->_k_slotChangeDecorationPosition();
rkflxUnsubmitted
Done

Unrelated?

rkflx: Unrelated?
1638 1648​
1639 emit viewChanged(view); 1649​ emit viewChanged(view);
1640 1650​
1641 const int zoom = previewForcedToTrue ? (KIconLoader::SizeHuge - KIconLoader::SizeSmall + 1) * 100 / maxSize : d->iconSizeForViewType(view); 1651​ const int zoom = previewForcedToTrue ? (KIconLoader::SizeHuge - KIconLoader::SizeSmall + 1) * 100 / maxSize : d->iconSizeForViewType(view);
1642 1652​
1643 // this will make d->iconsZoom be updated, since setIconsZoom slot will be called 1653​ // this will make d->iconsZoom be updated, since setIconsZoom slot will be called
1644 emit currentIconSizeChanged(zoom); 1654​ emit currentIconSizeChanged(zoom);
1645} 1655​}
▲ Show 20 Lines • Show All 465 Lines • ▼ Show 20 Line(s)2120 } else if (viewStyle == QLatin1String("DetailTree")) {
2111 d->defaultView |= KFile::DetailTree; 2121​ d->defaultView |= KFile::DetailTree;
2112 } else { 2122​ } else {
2113 d->defaultView |= KFile::Simple; 2123​ d->defaultView |= KFile::Simple;
2114 } 2124​ }
2115 //if (configGroup.readEntry(QLatin1String("Separate Directories"), 2125​ //if (configGroup.readEntry(QLatin1String("Separate Directories"),
2116 // DefaultMixDirsAndFiles)) { 2126​ // DefaultMixDirsAndFiles)) {
2117 // d->defaultView |= KFile::SeparateDirs; 2127​ // d->defaultView |= KFile::SeparateDirs;
2118 //} 2128​ //}
2119 if (configGroup.readEntry(QStringLiteral("Show Preview"), false)) { 2129​ 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…
2120 d->defaultView |= KFile::PreviewContents; 2130​ d->defaultView |= KFile::PreviewContents;
2121 } 2131​ }
2122 2132​
2123 d->previewWidth = configGroup.readEntry(QStringLiteral("Preview Width"), 100); 2133​ d->previewWidth = configGroup.readEntry(QStringLiteral("Preview Width"), 100);
2124 2134​
2125 if (configGroup.readEntry(QStringLiteral("Show hidden files"), 2135​ if (configGroup.readEntry(QStringLiteral("Show hidden files"),
2126 DefaultShowHidden)) { 2136​ DefaultShowHidden)) {
2127 d->actionCollection->action(QStringLiteral("show hidden"))->setChecked(true); 2137​ d->actionCollection->action(QStringLiteral("show hidden"))->setChecked(true);
Show All 17 Lines2154 } else if (sortBy == QLatin1String("Type")) {
2145 sorting |= QDir::Type; 2155​ sorting |= QDir::Type;
2146 } 2156​ }
2147 if (configGroup.readEntry(QStringLiteral("Sort reversed"), DefaultSortReversed)) { 2157​ if (configGroup.readEntry(QStringLiteral("Sort reversed"), DefaultSortReversed)) {
2148 sorting |= QDir::Reversed; 2158​ sorting |= QDir::Reversed;
2149 } 2159​ }
2150 d->updateSorting(sorting); 2160​ d->updateSorting(sorting);
2151 2161​
2152 if (d->inlinePreviewState == Private::NotForced) { 2162​ if (d->inlinePreviewState == Private::NotForced) {
2153 d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), false); 2163​ d->showPreviews = configGroup.readEntry(QStringLiteral("Previews"), false);
2164​ d->showPreviewsConfigEntry = d->showPreviews;
ngrahamUnsubmitted
Done

Unintentional changes from D12328

ngraham: Unintentional changes from D12328
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 } 2165​ }
2155 QStyleOptionViewItem::Position pos = (QStyleOptionViewItem::Position) configGroup.readEntry(QStringLiteral("Decoration position"), (int) QStyleOptionViewItem::Left); 2166​ QStyleOptionViewItem::Position pos = (QStyleOptionViewItem::Position) configGroup.readEntry(QStringLiteral("Decoration position"), (int) QStyleOptionViewItem::Left);
2156 setDecorationPosition(pos); 2167​ setDecorationPosition(pos);
2157} 2168​}
2158 2169​
2159void KDirOperator::writeConfig(KConfigGroup &configGroup) 2170​void KDirOperator::writeConfig(KConfigGroup &configGroup)
2160{ 2171​{
2161 QString sortBy = QStringLiteral("Name"); 2172​ QString sortBy = QStringLiteral("Name");
▲ Show 20 Lines • Show All 47 Lines • ▼ Show 20 Line(s)
2209 } else if (KFile::isTreeView(fv)) { 2220​ } else if (KFile::isTreeView(fv)) {
2210 style = QStringLiteral("Tree"); 2221​ style = QStringLiteral("Tree");
2211 } else if (KFile::isDetailTreeView(fv)) { 2222​ } else if (KFile::isDetailTreeView(fv)) {
2212 style = QStringLiteral("DetailTree"); 2223​ style = QStringLiteral("DetailTree");
2213 } 2224​ }
2214 configGroup.writeEntry(QStringLiteral("View Style"), style); 2225​ configGroup.writeEntry(QStringLiteral("View Style"), style);
2215 2226​
2216 if (d->inlinePreviewState == Private::NotForced) { 2227​ if (d->inlinePreviewState == Private::NotForced) {
2217 configGroup.writeEntry(QStringLiteral("Previews"), d->showPreviews); 2228​ configGroup.writeEntry(QStringLiteral("Previews"), d->showPreviewsConfigEntry);
ngrahamUnsubmitted
Done

Unintentional changes from D12328

ngraham: Unintentional changes from D12328
2218 if (qobject_cast<QListView *>(d->itemView)) { 2229​ if (qobject_cast<QListView *>(d->itemView)) {
2219 configGroup.writeEntry(QStringLiteral("listViewIconSize"), d->iconsZoom); 2230​ configGroup.writeEntry(QStringLiteral("listViewIconSize"), d->iconsZoom);
2220 } else { 2231​ } else {
2221 configGroup.writeEntry(QStringLiteral("detailedViewIconSize"), d->iconsZoom); 2232​ configGroup.writeEntry(QStringLiteral("detailedViewIconSize"), d->iconsZoom);
2222 } 2233​ }
2223 } 2234​ }
2224 2235​
2225 configGroup.writeEntry(QStringLiteral("Decoration position"), (int) d->decorationPosition); 2236​ configGroup.writeEntry(QStringLiteral("Decoration position"), (int) d->decorationPosition);
▲ Show 20 Lines • Show All 333 Lines • ▼ Show 20 Line(s)2532{
2559 } 2570​ }
2560} 2571​}
2561 2572​
2562void KDirOperator::Private::_k_slotItemsChanged() 2573​void KDirOperator::Private::_k_slotItemsChanged()
2563{ 2574​{
2564 completeListDirty = true; 2575​ completeListDirty = true;
2565} 2576​}
2566 2577​
2578​void KDirOperator::Private::updatePreviewActionState()
2579​{
2580​ if (!itemView) {
2581​ return;
2582​ }
2583​
2584​ const QFontMetrics metrics(itemView->viewport()->font());
2585​
2586​ // hide icon previews when they are too small
2587​ const bool iconSizeBigEnoughForPreview = itemView->iconSize().height() > metrics.height() * 2;
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…
2588​
2589​ KToggleAction *previewAction = qobject_cast<KToggleAction *>(actionCollection->action(QStringLiteral("inline preview")));
elvisangelaccioUnsubmitted
Done

qobject_cast

elvisangelaccio: `qobject_cast`
2590​ previewAction->setEnabled(iconSizeBigEnoughForPreview);
2591​
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…
2592​ if (iconSizeBigEnoughForPreview) {
2593​ previewAction->setToolTip(i18n("Show Preview"));
2594​ } else {
2595​ previewAction->setToolTip(i18n("Automatically disabled for small icon sizes; increase icon size to see previews"));
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…
2596​ }
2597​
2598​ calledFromUpdatePreviewActionState = true;
2599​ previewAction->setChecked(iconSizeBigEnoughForPreview && showPreviewsConfigEntry);
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…
2600​ calledFromUpdatePreviewActionState = false;
2601​}
2602​
2567void KDirOperator::Private::updateListViewGrid() 2603​void KDirOperator::Private::updateListViewGrid()
2568{ 2604​{
2569 if (!itemView) { 2605​ if (!itemView) {
2570 return; 2606​ return;
2571 } 2607​ }
2572 2608​
2609​ updatePreviewActionState();
2610​
2573 QListView *view = qobject_cast<QListView *>(itemView); 2611​ QListView *view = qobject_cast<QListView *>(itemView);
rkflxUnsubmitted
Done

Unrelated change.

rkflx: Unrelated change.
2574 2612​
2575 if (!view) { 2613​ if (!view) {
2576 return; 2614​ return;
2577 } 2615​ }
2578 2616​
2579 const bool leftChecked = actionCollection->action(QStringLiteral("decorationAtLeft"))->isChecked(); 2617​ const bool leftChecked = actionCollection->action(QStringLiteral("decorationAtLeft"))->isChecked();
2580 2618​
2581 if (leftChecked) { 2619​ if (leftChecked) {
2582 view->setGridSize(QSize()); 2620​ view->setGridSize(QSize());
2583 KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate()); 2621​ KFileItemDelegate *delegate = qobject_cast<KFileItemDelegate *>(view->itemDelegate());
2584 if (delegate) { 2622​ if (delegate) {
2585 delegate->setMaximumSize(QSize()); 2623​ delegate->setMaximumSize(QSize());
2586 } 2624​ }
2587 } else { 2625​ } else {
2588 const QFontMetrics metrics(itemView->viewport()->font()); 2626​ const QFontMetrics metrics(itemView->viewport()->font());
rkflxUnsubmitted
Done

Unrelated change.

rkflx: Unrelated change.
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…
2589 2627​
2590 const int height = itemView->iconSize().height() + metrics.height() * 2.5; 2628​ const int height = itemView->iconSize().height() + metrics.height() * 2.5;
2591 const int minWidth = qMax(height, metrics.height() * 5); 2629​ const int minWidth = qMax(height, metrics.height() * 5);
2592 2630​
2593 const int scrollBarWidth = itemView->verticalScrollBar()->sizeHint().width(); 2631​ const int scrollBarWidth = itemView->verticalScrollBar()->sizeHint().width();
2594 2632​
2595 // Subtract 1 px to prevent flickering when resizing the window 2633​ // Subtract 1 px to prevent flickering when resizing the window
2596 // For Oxygen a column is missing after showing the dialog without resizing it, 2634​ // For Oxygen a column is missing after showing the dialog without resizing it,
▲ Show 20 Lines • Show All 89 Lines • Show Last 20 Lines