D9987: Lessen log spam by not checking for existence of file with empty name

rkflx abandoned this revision. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R303 KInit REVISION DETAIL https://phabricator.kde.org/D9987 To: rkflx, #frameworks Cc: kde-frameworks-devel, dhaumann, michaelh, ngraham, bruns

D12130: Use the more user-friendly string "File type" in the save dialogs

rkflx resigned from this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12130 To: ngraham, #frameworks, #vdg, bruns, alexeymin, abetts Cc: kde-frameworks-devel, safaalfulaij, davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

D12545: Set focus on the filename line edit when a file is selected

rkflx resigned from this revision. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12545 To: anemeth, #frameworks, #vdg, ngraham Cc: kde-frameworks-devel, rkflx, ngraham, #frameworks, michaelh, bruns

D12327: Show Detailed Tree View by default

rkflx resigned from this revision. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12327 To: ngraham, #frameworks, #vdg Cc: kde-frameworks-devel, elvisangelaccio, abetts, #frameworks, michaelh, ngraham, bruns

D12591: KFileWidget: Provide faster access to the icon position setting

rkflx abandoned this revision. Herald added a subscriber: kde-frameworks-devel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12591 To: rkflx, #frameworks, ngraham Cc: kde-frameworks-devel, ngraham, elvisangelaccio, michaelh, bruns

D7010: KSqueezedTextLabel: call updateGeometry() when text changes

rkflx resigned from this revision. REVISION DETAIL https://phabricator.kde.org/D7010 To: brauch, cfeck Cc: dhaumann, aacid, #frameworks, michaelh, ngraham, bruns

D12647: Move the inline preview button into the menu

rkflx resigned from this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12647 To: ngraham, #frameworks Cc: kde-frameworks-devel, anemeth, michaelh, ngraham, bruns

D14201: Set system default shortcut Ctrl+0 for "Actual Size" action

rkflx accepted this revision. rkflx added a comment. @muhlenpfordt I'd say go ahead with landing (after updating the version). It would be good to still have some testing time before the next Frameworks release. (The new Neon seems to be just around the corner, AFAICS.) REPOSITORY

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

rkflx accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH kpropsdlg REVISION DETAIL https://phabricator.kde.org/D14662 To: dfaure, rkflx, shubham, ngraham Cc: bruns, michaelh, kde-frameworks-devel, ngraham

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

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,

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

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,

D14449: Modify device usage information

rkflx added a comment. In D14449#303766 , @shubham wrote: > In D14449#300327 , @ngraham wrote: > > > +1 for using the same label but putting the information on more than one line like @rkflx

D14449: Modify device usage information

rkflx added a comment. > This comment has been deleted. You are doing this in every other Diff, which is a bit annoying. Please first check what you wrote, then click on Send. Otherwise people will just ignore notifications. > is there any other simpler way to achieve this?

D14449: Modify device usage information

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. Thanks, looking better than before now. There are still some improvements you could make: - There is a superfluous space before the comma in the second line. - I'd prefer the

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

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 > +

D14449: Modify device usage information

rkflx added a comment. In D14449#303660 , @shubham wrote: > > Let me plug my suggestion again: > > > > Device capacity: ==- 24% used of 94.4 Gib > >22.5 GiB used, 71.9 GiB free > > > > if this design is

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

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

D14501: Top-align labels in properties dialog

rkflx added a comment. Just realized this actually fixes a problem present in KDE 3 already (KDE 2 did not have the second line yet, as far as I could tell in a short but very interesting test). In D14501#301044 , @cfeck wrote: > Bonus

D14501: Top-align labels in properties dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R241:9829d0ef9e74: Top-align labels in properties dialog (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D14501?vs=38811=38865 REVISION DETAIL

D14501: Top-align labels in properties dialog

rkflx created this revision. rkflx added reviewers: dhaumann, ngraham. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: kde-frameworks-devel. rkflx requested review of this revision. REVISION SUMMARY The General tab of KIO's properties dialog shows

D14449: Modify device usage information

rkflx added a comment. In D14449#300841 , @dhaumann wrote: > Device capacity: 94.4 Gib > Device usage:==-- 24% full (71.9 GiB free, 22.5 GiB used) > > > Could you give this a try with screenshot? I'm afraid the

D14449: Display used space in GiB also

rkflx added a comment. In D14449#300327 , @ngraham wrote: > Here's another idea for how to express everything while remaining compact in the horizontal dimension: > > Device capacity: -- 24% full >22.5 GiB

D14449: Display used space in GiB also

rkflx added a comment. In D14449#300086 , @ngraham wrote: > Device capacity: 94.4 Gib > Device usage:==-- 24% full (71.9 GiB free, 22.5 GiB used) > That's much better. Another proposal built on top of that: Device

D14201: Set system default shortcut Ctrl+0 for "Actual Size" action

rkflx added a comment. Thanks for your support ;) In D14201#296520 , @ngraham wrote: > Let's hold off on landing this patch (if accepted by everyone) until Cirkuit has had a release with D14202 so we

D14201: Set system default shortcut Ctrl+0 for "Actual Size" action

rkflx added a comment. @ngraham Would this patch be something you could back? REPOSITORY R237 KConfig REVISION DETAIL https://phabricator.kde.org/D14201 To: muhlenpfordt, dfaure, broulik, ngraham Cc: rkflx, kde-frameworks-devel, michaelh, ngraham, bruns

D14161: [KStyle] Use dialog-question for question icon

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. Thanks for noticing, LGTM and restores the proper icon indeed. > This was changed in `KMessageBox` in 9b05a12a43dbe2b31a4655052f20de0832ac7a61

D14160: [KMessageBox] Call style for icon

rkflx added a comment. > Dialog icons are correct on Linux with Breeze …if you also apply D14161 . Otherwise LGTM. > This needs testing on Windows and Mac You could add or ping more reviewers who happen to have a dev setup on those

D12218: Remove Reload button from the file dialogs' toolbar

rkflx added a comment. In D12218#271290 , @davidedmundson wrote: > > It can be useful for network views, but those are a minority of use cases > > Every large scale KDE deployment (from my POV by far our most important userbase) has a

D12756: [KDateTable] Use more appropriate and readable text colors for weekends and holidays

rkflx added a comment. In D12756#268345 , @ngraham wrote: > Use conditional grays instead of reds (red was never an appropriate color anyway) Hm, now Sundays look like they belong to the previous/next month. (Do we even need to change

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In my personal experience spending more time on a limited set of items leads to better results and less churn (as in reverting commits, number of iterations per patch etc.) than only lightly touching a large amount of issues everywhere. Of course you can do whatever

D12858: [KCharSelect] Fix table cell size

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. Thanks, works great now! REPOSITORY R236 KWidgetsAddons REVISION DETAIL https://phabricator.kde.org/D12858 To: cfeck, #frameworks, rkflx Cc: rkflx, elvisangelaccio,

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. > The value is in standardizing on something. Personally I'd be okay with a consistent style per application, with the new style applied to new applications, and allowing exceptions where it makes sense. You've already witnessed the amount of bikeshedding in

D12130: Use the more user-friendly string "File type" in the save dialogs

rkflx added a comment. @ngraham I guess you are still working on this, or did you move on to other things in the meantime? (And please set the status to "Changes planned", so this does not clog up the review queue). REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12130

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In D12986#265666 , @ngraham wrote: > In D12986#265662 , @rkflx wrote: > > > Anyway, I'm not complaining too much (but please be aware that every change you propose creates

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. Thanks, "Size" and "Full trash" are looking good now. In D12986#265235 , @rkflx wrote: > In D12986#265231 , @ngraham wrote: > > > Yeah, that makes sense. Suggestions

D12858: [KCharSelect] Fix table cell width

rkflx added a comment. Not sure whether that's material for this patch, but for me the vertical spacing is still a bit off, i.e. the boxes are still not square as before: Qt 5.10: F5859388: kcharselect-qt5.10.png Qt 5.11: F5859390:

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. In D12986#265231 , @ngraham wrote: > Yeah, that makes sense. Suggestions welcome, of course. Is this any better? > F5859259: After 2.png Hm, I think after the checkbox there

D12986: [Trash KCM] Clean up and standardize UI to be in line with the KDE HIG

rkflx added a comment. I don't think putting whole phrases as a label before the checkbox works very well, and I would be surprised if that's what the HIG now recommends. I assume at most a single subject should be used there? For example, instead of the awkwardly placed checkbox in

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. Let's commit this on a separate branch, as getting micro-approvals from #Frameworks does not work that well. Then we can implement (in the form of reviewed Diffs as before) what we already discussed in a bigger setting,

D12734: Scale up folder icon before creating preview overlays

rkflx added a comment. In D12734#262368 , @muhlenpfordt wrote: > Ok to commit? Sure, go ahead  REPOSITORY R320 KIO Extras BRANCH scale-up-folder-icon (branched from Applications/18.04) REVISION DETAIL

D12848: Set fix steps for icon sizes

rkflx added a comment. > In the open/save dialog use the same fixed icon sizes for the zoom slider as Dolphin. So we could also change Dolphin to support continuous zoom instead? As the summary is quite sparse, I'm not sure what the patch is about: Is it about consistency, or is there

D12848: Set fix steps for icon sizes

rkflx added a comment. In D12848#262008 , @broulik wrote: > I would vote for removing the buttons We already discussed this and decided to keep the buttons. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12848 To:

D12130: Use the more user-friendly string "File type" in the save dialogs

rkflx added a comment. Restricted Application added a subscriber: kde-frameworks-devel. In D12130#250478 , @ngraham wrote: > Yes, and I did more investigation myself, and all the glue is in plasma-integration. Some changes might be needed there

D12848: Set fix steps for icon sizes

rkflx added a comment. As mentioned in the task: In T8552#139581 , @rkflx wrote: > > In T8552#139578 , @anemeth wrote: > > > >> Could we have the same icon zoom slider as in Dolphin? > >> >

D12538: Allow accepting by double-click in save dialog

rkflx accepted this revision. This revision is now accepted and ready to land. REPOSITORY R241 KIO BRANCH arcpatch-D12538 (branched from master) REVISION DETAIL https://phabricator.kde.org/D12538 To: anemeth, #frameworks, #vdg, ngraham, rkflx Cc: kde-frameworks-devel, ltoscano, rkflx,

D12538: Allow accepting by double-click in save dialog

rkflx added inline comments. INLINE COMMENTS > ngraham wrote in kfilewidget.cpp:1866 > Not for everything, just this one. Well, how should that line look like then? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12538 To: anemeth, #frameworks, #vdg, ngraham, rkflx Cc:

D12538: Allow accepting by double-click in save dialog

rkflx added inline comments. INLINE COMMENTS > ngraham wrote in kfilewidget.cpp:1866 > Let's use new-style signal/slot syntax for new code. See > https://wiki.qt.io/New_Signal_Slot_Syntax Is this even possible without a lot of refactoring (which would be out-of-scope for this patch)? >

D12734: Scale up folder icon before creating preview overlays

rkflx added a comment. In D12734#259445 , @muhlenpfordt wrote: > I wanted to be careful here since I'm a bit afraid of changing library functions. If you think it's ok for `Applications/18.04` I'll rebase it. How else would you fix

D12734: Scale up folder icon before creating preview overlays

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. +1, but I'd feel a bit uncomfortable (almost) self-approving my own patch (sans the `qMax`) here, so perhaps wait until the end of the week for any other opinions before committing.

D12077: Show view mode buttons in the open/save dialog's toolbar

rkflx added a comment. One more idea: Would it make sense to land each part of the rework on a dedicated rework branch first, and only once everything fits together (both in design and implementation) merge that branch to master? This would avoid huge dependency stacks as well as the

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx accepted this revision. rkflx added a comment. This revision is now accepted and ready to land. Thanks, LGTM now. INLINE COMMENTS > ngraham wrote in kdiroperator.cpp:1888 > The problem conceptually is that `view-sort-ascending` is semantically > inaccurate for anything but ascending

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added inline comments. INLINE COMMENTS > kdiroperator.cpp:1888 > d->actionCollection->addAction(QStringLiteral("sorting menu"), > sortMenu); > +sortMenu->setIcon(QIcon::fromTheme(QStringLiteral("itemize"))); > While we agreed upon wanting a better icon, until that's done I'd

D12647: Move the inline preview button into the menu

rkflx added a comment. In T8552#140332 , @rkflx wrote: > > moving the Show Preview button to the menu > > Makes sense to me, but might be controversial with others. FWIW, if people do not like this (e.g. because they often toggle

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. In D12337#256813 , @ngraham wrote: > the context menu will have "Sorting > Sort by name. Thoughts? Actually I'd still find that kind of acceptable. REPOSITORY R241 KIO REVISION DETAIL

D12337: Give the file dialogs a "Sort by" menu button on the toolbar

rkflx added a comment. In D12337#256813 , @ngraham wrote: > Now we have a problem. With no text in the toolbar button, the actual items don't reveal their purpose so readily unless we change all of their strings to say "Sort by Name" (etc.).

D12321: Hide file preview when icon is too small

rkflx added a comment. In D12321#256797 , @anemeth wrote: > In D12321#256791 , @ngraham wrote: > > > Reverted. Alex, now that you have a fancy contributor account, you can land this yourself on

D12321: Hide file preview when icon is too small

rkflx added a comment. In D12321#256786 , @ngraham wrote: > Darn, my bad. Could we revert just the tooltip string change and land that for 5.47, or would you prefer to hold the whole feature until 5.47? Well, the tooltip was added for a

D12321: Hide file preview when icon is too small

rkflx added a comment. In D12321#253903 , @rkflx wrote: > Cool, LGTM now. Due to the string change this will have to wait for 5.47 ...and I even added a warning :/ REPOSITORY R241 KIO REVISION DETAIL

D12321: Hide file preview when icon is too small

rkflx added a comment. This breaks the string freeze. Please revert. See https://community.kde.org/Schedules/Frameworks. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12321 To: anemeth, #vdg, #frameworks, ngraham, rkflx, #dolphin, elvisangelaccio Cc:

D12594: KFileWidget: Perfectly align filename widget with icon view

This revision was automatically updated to reflect the committed changes. Closed by commit R241:d5f67b07b5ce: KFileWidget: Perfectly align filename widget with icon view (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12594?vs=33276=33448

D12593: KFileWidget: Save places panel width also after hiding panel

This revision was automatically updated to reflect the committed changes. Closed by commit R241:0bcc7eadf5ca: KFileWidget: Save places panel width also after hiding panel (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12593?vs=33275=33447

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

This revision was automatically updated to reflect the committed changes. Closed by commit R241:302c70b39723: KFileWidget: Prevent places panel width from growing 1px iteratively (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D12590: KFileWidget: Disable zoom buttons once reached minimum or maximum

This revision was automatically updated to reflect the committed changes. Closed by commit R241:00d8a78f5696: KFileWidget: Disable zoom buttons once reached minimum or maximum (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D12588: KFileWidget: Set minimum size for zoom slider

This revision was automatically updated to reflect the committed changes. Closed by commit R241:e4f4c66f96d8: KFileWidget: Set minimum size for zoom slider (authored by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12588?vs=33271=33442 REVISION DETAIL

D12545: Set focus on the filename line edit when a file is selected

rkflx added a comment. In D12545#256043 , @anemeth wrote: > I'm thinking about abandoning this revision in favor of proper tab ordering instead. As Nate already said, tabstop ordering would also be good to have. But given that the

D12077: Show view mode buttons in the open/save dialog's toolbar

rkflx added a comment. I wonder if we should revert this before it ships to users. I'd rather not introduce this in 5.46 if we are about to change it again for 5.47. Better accumulate visual changes a bit, because constantly changing the dialog might get annoying for users. REPOSITORY

D12590: KFileWidget: Disable zoom buttons once reached minimum or maximum

rkflx updated this revision to Diff 33367. rkflx added a comment. - Rename actions too REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12590?vs=33272=33367 BRANCH disable-zoom-buttons (branched from master) REVISION DETAIL

D12591: KFileWidget: Provide faster access to the icon position setting

rkflx planned changes to this revision. rkflx added a comment. In D12591#255639 , @elvisangelaccio wrote: > > While the submenu itself is not advertised in the API docs, it was publicly accessible > > Where? Isn't it a member of

D12538: Allow accepting by double-click in save dialog

rkflx added a comment. In D12538#255954 , @anemeth wrote: > In D12538#254871 , @rkflx wrote: > > > However, for double click mode and Save, descending into directories is kinda broken. You might

D12538: Allow accepting by double-click in save dialog

rkflx retitled this revision from "Always prompt for overwrite on double-click in save dialog" to "Allow accepting by double-click in save dialog". rkflx edited the summary of this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12538 To: anemeth, #frameworks,

D12591: KFileWidget: Provide faster access to the icon position setting

rkflx added a dependent revision: D12598: FileView: Provide faster access to the icon position setting. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12591 To: rkflx, #frameworks Cc: michaelh, bruns

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

rkflx added a comment. Sorry for the wait, writing the commit message for this one and the alignment patch meant going through a //lot// of notes. Who'd knew that after the off-by-one error in Spectacle which only happened in a VM there could be even more difficult problems :D REPOSITORY

D12591: KFileWidget: Provide faster access to the icon position setting

rkflx added a comment. If this gets approved, I'll do the same change to K3b for consistency, which was the only other app showing the submenu I could find on https://lxr.kde.org. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12591 To: rkflx, #frameworks Cc:

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

rkflx added a dependent revision: D12594: KFileWidget: Perfectly align filename widget with icon view. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12592 To: rkflx, #frameworks Cc: michaelh, bruns

D12594: KFileWidget: Perfectly align filename widget with icon view

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY Since the redesign of the file dialog for KDE4, the left edges of the line edits below the icon view were supposed to

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

rkflx added a dependent revision: D12593: KFileWidget: Save places panel width also after hiding panel. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12592 To: rkflx, #frameworks Cc: michaelh, bruns

D12593: KFileWidget: Save places panel width also after hiding panel

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY In certain situations the user-set width of the places panel was not remembered. As `placesViewWidth` is

D12592: KFileWidget: Prevent places panel width from growing 1px iteratively

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY cc00bbb22010 tried to prevent the places

D12591: KFileWidget: Provide faster access to the icon position setting

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY The file dialog allows to move the file name labels from next to the icon to right under the icon for Short View.

D12590: KFileWidget: Disable zoom buttons once reached minimum or maximum

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY In Gwenview the zoom buttons will be disabled when clicking on it would not result in any further change of zoom

D12588: KFileWidget: Set minimum size for zoom slider

rkflx created this revision. rkflx added a reviewer: Frameworks. Restricted Application added a project: Frameworks. rkflx requested review of this revision. REVISION SUMMARY When choosing a very narrow width for the file dialog, the zoom slider for the icon size would get squished so that

D12538: Always prompt for overwrite on double-click in save dialog

rkflx added a comment. TBH, I don't get what the overwrite and release schedule discussion has to do with this patch. All it does is add an additional method to trigger `slotOk()`, i.e. instead of clicking on Save, users can simply double-click. The fact that either the dialog or the

D12538: Select item without clicking the Open/Save button

rkflx added a comment. > [remove] BUG: 267749 Now I'm confused: Isn't what we do here exactly what the bug reporter wanted? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12538 To: anemeth, #frameworks, #vdg, ngraham, rkflx Cc: rkflx, broulik, jtamate, ngraham,

D12538: Select item without clicking the Open/Save button

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. Thanks, works great now for single click users. Glad we could skip a huge discussion ;) However, for double click mode and Save, descending into directories is kinda broken.

D12544: Don't select file extension

rkflx accepted this revision. rkflx added a comment. Thanks for the patch. Pondered over the code, but could not find anything wrong. In D12544#254554 , @anemeth wrote: > I removed these checks: `&& !locationEdit->isVisible()` > I assume

D12545: Set focus on the filename line edit when a file is selected

rkflx edited the summary of this revision. rkflx edited the test plan for this revision. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D12545 To: anemeth, #frameworks, #vdg, ngraham, rkflx Cc: rkflx, ngraham, #frameworks, michaelh, bruns

D12538: Select item without clicking the Open/Save button

rkflx added a comment. > https://bugs.kde.org/show_bug.cgi?id=267749 Wow, reading the bug again, my suggestions is fitting right in! Double click to trigger Save for single click users. Only after some confusion in Comment 1 the only slightly related question of overwriting came up in

D12538: Select item without clicking the Open/Save button

rkflx added a comment. After sleeping over this, I don't feel comfortable the way this is supposed to work for single click: Selection markers only really make sense when enabling multi-select mode. Abusing a single selection marker in single-select mode to mean "put the filename in the

D12328: Enable preview by default in the filepicker dialog

rkflx accepted this revision. rkflx added a subscriber: elvisangelaccio. rkflx added a comment. This revision is now accepted and ready to land. LGTM now (even though you ignored the "newline" I suggested…) I'd appreciate a second opinion from #Frameworks

D12545: Set focus on the filename line edit when a file is selected

rkflx added a comment. In D12545#254479 , @ngraham wrote: > No, it doesn't actually work. Turn on double-click, Aha! This is what I'm missing. It should be mentioned in the summary if non-default settings are used :D REPOSITORY R241

D12545: Set focus on the filename line edit when a file is selected

rkflx added a comment. In D12545#254475 , @ngraham wrote: > Good point @rkflx, we definitely don't want to break keyboard navigation. We want this behavior only when clicking, and only for the save dialog. Yeah, but not sure I

D12545: Set focus on the filename line edit when a file is selected

rkflx requested changes to this revision. rkflx added a comment. Hm, this breaks selecting files and even navigating directories with the keyboard (e.g. via the arrow keys), and as such cannot possibly be something we want. I could imagine a different spin here: [⇥] should switch focus

D12328: Enable preview by default in the filepicker dialog

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. Thanks, `arc patch` now works great for me, automatically applying the dependent revision as it should. In D12328#254098 , @anemeth

D12385: Thumbnail smooth scaling in filepicker

rkflx accepted this revision. rkflx added a comment. Thanks Kai, great questions. I think I can answer them: In D12385#250511 , @broulik wrote: > What's the performance penalty of that? I don't see any performance issues, both when

D12321: Hide file preview when icon is too small

rkflx accepted this revision. rkflx added a subscriber: elvisangelaccio. rkflx added a comment. Cool, LGTM now. Due to the string change this will have to wait for 5.47, and we have to find a general consensus in #frameworks about the nature of

D12328: Enable preview by default in the filepicker dialog

rkflx added a comment. Thanks for the update, but please make sure not to lose the `kconf_update` changes… INLINE COMMENTS > kdiroperator.cpp:2163 > +d->showPreviews = configGroup.readEntry(QStringLiteral("Show Inline > Previews"), true); > +d->showPreviewsConfigEntry =

D12321: Hide file preview when icon is too small

rkflx added a comment. Thanks. Still not 100% there, though ;) (And please rebase on current master, while you are at it…) INLINE COMMENTS > kdiroperator.cpp:2622 > } else { > -const QFontMetrics metrics(itemView->viewport()->font()); > +const QFontMetrics

D12306: Improve grid icon layout in filepicker dialog

This revision was automatically updated to reflect the committed changes. Closed by commit R241:bbc262674a56: Improve grid icon layout in filepicker dialog (authored by anemeth, committed by rkflx). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D12321: Hide file preview when icon is too small

rkflx requested changes to this revision. rkflx added a comment. @anemeth Finally got to the code review part. First of all, thanks for implementing the disabled/turned-off behaviour I suggested, works really great now and surely makes the feature much more pleasant to use. Some small

D12328: Enable preview by default in the filepicker dialog

rkflx requested changes to this revision. rkflx added a comment. This revision now requires changes to proceed. In D12328#252246 , @rkflx wrote: > In D12328#250028 , @anemeth wrote: > > > Rebase on

  1   2   3   >