huoni (Huon Imberger)
User

Projects

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Friday

  • Clear sailing ahead.

User Details

User Since
Feb 2 2018, 4:19 AM (37 w, 4 d)
Availability
Available

Recent Activity

Sep 16 2018

huoni abandoned D11008: Fix issues regarding available operations being out of sync with selected image.

No time to look at this :(

Sep 16 2018, 12:26 AM · Gwenview

Jul 9 2018

huoni added a comment to D13723: Fix issues with cursors and unwanted actions when right-clicking.

It might be that the description in my summary was a little misleading, I adapted it slightly now.

Jul 9 2018, 9:35 AM

Jul 8 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

Finally got around to summarize the cursor issues:

  1. When moving very slowly while initiating a drag, the ForbiddenCursor would appear briefly until you moved the mouse further along. I think this is an issue in Qt, as we can observe the same behaviour in Dolphin. In QBasicDrag::drag, DragCopyCursor is set, then QBasicDrag::updateCursor changes the cursor to ForbiddenCursor, until finally QBasicDrag::updateCursor updates to the final cursor, e.g. DragCopyCursor. We can avoid the flickering by always displaying ForbiddenCursor (see next point).
  2. https://bugs.kde.org/show_bug.cgi?id=386034 proposed that "the image should not be able to be dropped onto itself.", which your patch still allows. As that's actually more related to 131d25855e11, I'll simply fix this in a separate Diff, see D13724.
  3. ClosedHandCursor shown when trying to drop to Dolphin: Turns out that's the correct cursor for the default case of moving objects, which is set implicitly by QGraphicsView. For dragging from the ThumbnailView it works fine, because there copying is set as the default action. That's where there actually is a difference to what your patch is implementing ;) In D13725 I'm doing some further refinements to the dragging of thumbnails, and replicate that change for dragging from the canvas too, which should give us the more pleasant DragCopyCursor.
  4. Not strictly related, but as I mentioned it above: The spurious ClosedHandCursor after showing the context menu will be fixed with D13723.
  5. Slowly starting to drag so the ClosedHandCursor shows, the ClosedHandCursor is shown after the drag. Again, here I believe the issue might be in Qt. qDebug() shows that the cursor is correctly (re)set, but somehow the change takes effect only after moving the mouse again. Workaround which seems to fix the problem in D13726.
Jul 8 2018, 12:47 AM · Gwenview
huoni accepted D13723: Fix issues with cursors and unwanted actions when right-clicking.
Jul 8 2018, 12:42 AM
huoni added a comment to D13723: Fix issues with cursors and unwanted actions when right-clicking.

cursor/fix-context-menu (branched from Applications/18.04)

Friendly reminder that tagging is on Monday at 23:59 UTC.

Jul 8 2018, 12:42 AM
huoni accepted D13726: Avoid cursor glitch after trying to drag an image onto itself.

Nice, such a simple fix!

Jul 8 2018, 12:28 AM
huoni accepted D13725: Show more appropriate cursor when dragging and enable modifier keys.

Two things I noticed while testing:

Jul 8 2018, 12:19 AM
huoni accepted D13724: Do not allow to drag an image onto itself.

Sorry for the delay!

Jul 8 2018, 12:04 AM

Jun 24 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

Final polishing LGTM. I'll submit the cursor fixes in separate Diffs, don't worry about it ;) Thanks again for the patch, and sorry it took so long to review…

Jun 24 2018, 1:28 AM · Gwenview
huoni committed R260:984b9737f079: Allow dragging from View mode to external applications (authored by huoni).
Allow dragging from View mode to external applications
Jun 24 2018, 1:28 AM
huoni closed D11877: Allow dragging from View mode to external applications.
Jun 24 2018, 1:28 AM · Gwenview

Jun 23 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

The cursor problem is kind of solved. There is still some flickering sometimes.

Nice. Let me look into this in the next days, then we can decide whether we can fix things now or simply open a bug to work on it later…

I identified 5 different cursor issues (and lots of unrelated deficiencies…), and I figured out how to fix 4 of them. I'll keep you posted.

Jun 23 2018, 5:15 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • resetDragDetection rename
Jun 23 2018, 5:14 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • const
  • better way of working with KFileItem/KFileItemList
  • lazy initialize thumbnail provider
  • check for mDrag closer to its usage
Jun 23 2018, 5:11 AM · Gwenview

Jun 10 2018

huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Remove redundent comments
  • Improve variable naming, code readability
Jun 10 2018, 12:15 AM · Gwenview

Jun 9 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

ThumbnailProvider worked nicely. I was initially put off by the complicated use in ThumbnailView which is why I didn't try it to begin with.

Jun 9 2018, 3:18 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Clean up KFileItemList code
Jun 9 2018, 3:13 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Change to using ThumbnailProvider to generate drag pix
  • Fix stuck cursor after dragging an image
Jun 9 2018, 3:04 AM · Gwenview

Jun 8 2018

huoni planned changes to D11877: Allow dragging from View mode to external applications.

Just one note regarding KIO::PreviewJob: This is only a small part of ThumbnailProvider, which seems to contain lots of additional code for special cases. For example this can be observed for fish://, where your approach fails to provide a pixmap, while you'd get a proper thumbnail when dragging from the Thumbnail Bar. I'm sure there are many more cases.

Would it be possible to create a new ThumbnailProvider upon initiating the drag, connect its thumbnailLoaded signal to setPixmap, and then appendItems? That would be the next thing I would try, as it uses proven code and avoids creating a thumbnail when not starting to drag at all.

Jun 8 2018, 11:06 PM · Gwenview

Jun 3 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

Thanks for the update ;)

Any ideas would be helpful.

I'll try to look into it later.

Jun 3 2018, 10:57 AM · Gwenview
huoni added a comment to D11877: Allow dragging from View mode to external applications.

I kept everything in DocumentView because I wanted to support dragging a video.

Jun 3 2018, 6:20 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Don't drag if document failed to load
  • Update to use MimeTypeUtils::selectionMimeData
  • Respect mimimum drag distance
  • Use DragPixmapGenerator
Jun 3 2018, 6:17 AM · Gwenview

Jun 2 2018

huoni added a comment to T6321: [WISH] Create a quick image editor for Spectacle.

I noticed a more general problem which might be related: Try drawing straight lines (modifier FTW ;) with 1/2/3/4px width next to each other and look at them with KMag. They are not really aligned to the pixel grid, so Qt uses anti-aliasing, resulting in different colours and line widths. I think that's important to solve first, and afterwards focus on polishing the rectangle outlines or try a different approach if that fails.

Here's a screenshot that shows this. All lines drawn at 1px setting. Actual width is 2px, and therefore opacity is also <100%.

Yes, Antialiasing is enabled on purpose because otherwise the line would look like this:


I'm not sure if there is much that can be done, we could try to disable Antialiasing below a specific width but IMO that looks worse.

Jun 2 2018, 11:41 PM · VDG, Spectacle
huoni committed R260:131d25855e11: Allow dragging files/folders to View mode (authored by huoni).
Allow dragging files/folders to View mode
Jun 2 2018, 11:31 PM
huoni closed D11879: Allow dragging files/folders to View mode.
Jun 2 2018, 11:31 PM · Gwenview

Jun 1 2018

huoni added a comment to D11879: Allow dragging files/folders to View mode.

But there's one problem I haven't been able to solve - accepting drops when a video is displayed. Something to do with QGraphicsProxyWidget not forwarding the events, or something along those lines. I'm not sure if this is a show stopper though, given Gwenview's primarily an image viewer, not a video viewer.

Hm, adding the corresponding override I could get it to show the "accept" cursor briefly, but then it crashed in some kind of endless event loop. Let's not worry about it.

Jun 1 2018, 11:33 PM · Gwenview
huoni updated the diff for D11879: Allow dragging files/folders to View mode.
  • const
  • Remove unnecessary QObject::
  • Call QGraphicsWidget instead of QGraphicsItem in drag events
  • Rebase
Jun 1 2018, 11:31 PM · Gwenview

May 27 2018

huoni added a comment to D11879: Allow dragging files/folders to View mode.

Maybe you ran into something like https://forum.qt.io/topic/3092/qgraphicsscene-drag-and-drop (relating to class AbstractImageView : public QGraphicsWidget)?

May 27 2018, 4:18 AM · Gwenview
huoni updated the diff for D11879: Allow dragging files/folders to View mode.
  • Handle drop events in DocumentView
  • Fix crash when dragging non-URL mimetypes
  • Fix compile error
May 27 2018, 4:18 AM · Gwenview

May 25 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

I think Document::mimeData could be a good spot?

Not really, because Document only knows about a single document, while we also need to select a list of URLs. Essentially we (at least currently) depend on ContextManager in the implementation, but that's quite hard to get to in ThumbnailView as I found out today. Fortunately, I'm not out of ideas yet, it will only take me more time than planned…

May 25 2018, 11:17 PM · Gwenview
huoni added a comment to D13010: Update thumbnail de-/select hover button on selection change.

@huoni Thanks for reviewing 👍

(Regarding P220, I just realized it would have been helpful to also dump this to prevent confusion… Sorry for that.)

May 25 2018, 11:00 PM
huoni planned changes to D11008: Fix issues regarding available operations being out of sync with selected image.

I no longer think getting rid of currentChanged is the right way to go about this, so I plan to have another look at it.

May 25 2018, 10:52 PM · Gwenview

May 24 2018

huoni added a comment to T6321: [WISH] Create a quick image editor for Spectacle.
In T6321#143745, @rkflx wrote:

pull the top handle down below the bottom handle

Spectacle and Inkscape (in that special resize mode you get into with double-clicking) simply stop resizing and don't invert the object or change the handle. I could imagine this would work well for us too (only for resizing, not for creation, of course). BTW, Shutter has the same problem…

May 24 2018, 10:19 AM · VDG, Spectacle
huoni accepted D13010: Update thumbnail de-/select hover button on selection change.
  • ThumbnailBar::selectionChanged is always triggered twice when selection changes. Seems to me there's some sort of selection changed event duplication happening somewhere

There are two different instances of ThumbnailBarView created in ViewMainPage and FullScreenContent, so the slot is called for both thumbnail bars.

  • Changing the bar selection in View mode seemingly triggers the Browse hover buttons to update, which is unnecessary

The selection events are sent to visible and not visible thumbnail view/bars (with or without this patch). Maybe we could prevent this, but then have to update the selection on mode change. I think this could get tricky.

May 24 2018, 12:01 AM

May 23 2018

huoni added a comment to D13010: Update thumbnail de-/select hover button on selection change.

If I see this correctly, the only time the buttons weren't updated was when the hover did not change, but the selection did. E.g. when Ctrl + clicking, or + arrow. Oops, just realised this was cleared up in the Test Plan.
If so, then the patch LGTM.

May 23 2018, 6:13 AM
huoni planned changes to D11879: Allow dragging files/folders to View mode.

Thanks for the patch :) From a behavioural standpoint it works very well, and you seem to have covered a lot of edge cases.

Nevertheless, I have a couple of points.


I'm not able to compile all of Gwenview:
part/gvpart.cpp: In constructor ‘Gwenview::GVPart::GVPart(QWidget*, QObject*, const QVariantList&)’:
part/gvpart.cpp:64:78: error: no matching function for call to ‘Gwenview::DocumentViewContainer::DocumentViewContainer(QWidget*&)’
     DocumentViewContainer* container = new DocumentViewContainer(parentWidget);
                                                                              ^

Note that this might indicate a larger problem with your patch, i.e. ViewMainPage might not be the best place to implement drop handling. Using Gwenview's KPart in Konqueror should also support dropping of URLs (at least in theory, not sure if this would need more work, so likely out-of-scope for this patch).

I don't see yet why you'd need to use an eventFilter and add a dependency on ViewMainPage. Normally reimplementing the corresponding event like it's done in ThumbnailView::dropEvent should be enough.

Qt's docs advice

To handle the incoming drag, reimplement QGraphicsItem::dragEnterEvent()

May 23 2018, 5:14 AM · Gwenview
huoni planned changes to D11877: Allow dragging from View mode to external applications.

Thanks for the feedback. I will wait until D13028 is finalised before doing anything. I also think waiting until we can put the mimedata code in a central place is a good idea. I think Document::mimeData could be a good spot?

May 23 2018, 4:42 AM · Gwenview

May 21 2018

huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Remove include that I missed
May 21 2018, 1:48 AM · Gwenview
huoni updated the diff for D11877: Allow dragging from View mode to external applications.
  • Use KIO::PreviewJob to generate pixmap thumbnail
  • Add (possibly edited) image data to the drag mimedata
May 21 2018, 1:45 AM · Gwenview
huoni added a comment to D11877: Allow dragging from View mode to external applications.
  • Dragging is already implemented in Browse mode and for the Thumbnail Bar. Is there any reason you are (partly) reinventing the wheel regarding the thumbnails?

Had a good look into whether we could leverage the existing thumbnail code, but found it impractical due to:

  1. The thumbnail view thumbnails are dependent on the directory model. If one opens Gwenview directly to View, the directory model isn't loaded before the image is shown
  2. The code is spread over multiple classes, there is no single place we can hook into.

Can we at least partly refactor and reuse some code? For example I noticed that compared to dragging thumbnails your code positions the drag pixmap differently and does not set a frame border. I assume we are both talking about DragPixmapGenerator?

May 21 2018, 1:43 AM · Gwenview

May 20 2018

huoni added a comment to D11877: Allow dragging from View mode to external applications.

Thanks for implementing this feature ;)

Before I look at the code in detail, some comments:

  • You might want to test whether (or rather how…) this interferes with Crop.
May 20 2018, 9:12 AM · Gwenview

May 19 2018

huoni added inline comments to D12871: Copy raster image and SVG content to clipboard.
May 19 2018, 8:51 AM

May 18 2018

huoni added a comment to D12871: Copy raster image and SVG content to clipboard.

D11877 (which FYI does not setImageData, but still works when dragging to Chromium and GIMP - exactly how Spectacle works).

Spectacle and your patch use setPixmap, which is not all that different from setImage (which internally calls setImageData), is it ;)

May 18 2018, 12:09 AM

May 17 2018

huoni added a comment to D12902: Update paste action on current directory and selection changes.

However, I notice that in Dolphin, pressing the shortcut (Ctrl+V) still results in an error dialog. So it's not the entire action that is disabled, just the menu item. We should copy that behaviour I think.

Strange - I can't reproduce this. Tried with current master and installed version 17.12.3.
I don't think this is possible with the standard QAction (if it's not a bug).
How did you provoke this error dialog?

May 17 2018, 6:26 AM

May 16 2018

huoni added a comment to D12902: Update paste action on current directory and selection changes.

Hm, seems like the original intention of this change (0861dd248c01) was to adapt to the Dolphin behaviour which in fact disables the paste action for non writable folders.

May 16 2018, 11:20 PM
huoni added inline comments to D12871: Copy raster image and SVG content to clipboard.
May 16 2018, 6:20 AM
huoni added a comment to D12902: Update paste action on current directory and selection changes.

The first option sounds a bit more handy to me.

May 16 2018, 5:58 AM
huoni added a comment to D12902: Update paste action on current directory and selection changes.

Patch works as advertised.

May 16 2018, 3:32 AM
huoni added a comment to D12871: Copy raster image and SVG content to clipboard.

All except LibreOffice Writer worked as expected (pasted the bitmap). Writer presumably used the URL since the original image was pasted.
It might be confusing in some situations, but I don't see how we could solve the issue without removing the URL from the mime data, which is a bad idea.

May 16 2018, 3:15 AM

May 15 2018

huoni added a comment to D12871: Copy raster image and SVG content to clipboard.

Or am I wrong?

May 15 2018, 5:50 AM

May 14 2018

huoni added a comment to D12871: Copy raster image and SVG content to clipboard.

I think we should change the current Copy action to copy the bitmap, but only in View mode (if we're in compare mode, then only the selected image). There is no ambiguity with multiple selection here.
I think when viewing a single image, Copy is naturally associated with copying the bitmap (not the file), so one can paste in a browser or image editor. But in Browse, the images are more like files, so copying there should behave more like a file (just like dragging and dropping).

May 14 2018, 11:38 PM

May 13 2018

huoni added a comment to D12824: Cleanup code for finishing undo/redo of image operations.

In terms of the Phabricator changes - what's the way for Gwenview?
Are there any effects when using #gwenview as reviewer?

As far as I understood, the change only affected Herald rules (which we don't have) and forwarding to mailinglists (which we don't do, and which I'd find quite questionable).

As long as nobody of us gets too annoyed with our current way of working, I'd say we simply carry on as usual (but I believe there are also more fine-grained filtering options available using Phab's mail headers, should the need arise to distinguish between subscriber/reviewer notifications and bulk Gwenview mail).

May 13 2018, 12:15 AM

May 9 2018

huoni committed R260:f0edb69f3063: Disable View mode shortcuts outside of View mode (authored by huoni).
Disable View mode shortcuts outside of View mode
May 9 2018, 3:55 AM
huoni added a comment to D11633: Disable View mode shortcuts outside of View mode.

Thanks, LGTM. I'd say this can go to the stable branch (18.04.1, if you are fast with committing…).

You could still cherry-pick to the stable branch, if you want the fix to be part of 18.04.2…

May 9 2018, 3:53 AM · Gwenview

May 8 2018

huoni committed R260:20b198b4fe52: Disable View mode shortcuts outside of View mode (authored by huoni).
Disable View mode shortcuts outside of View mode
May 8 2018, 1:10 AM
huoni closed D11633: Disable View mode shortcuts outside of View mode.
May 8 2018, 1:10 AM · Gwenview
huoni added a comment to D11633: Disable View mode shortcuts outside of View mode.

I did exactly that. Perhaps your file was named F.png, while mine read f.png?

May 8 2018, 1:08 AM · Gwenview

May 7 2018

huoni added a comment to D11633: Disable View mode shortcuts outside of View mode.

When starting in Browse, View actions such as Zoom to Fit are correctly ignored

Hm, I cannot confirm that part: Even starting in Browse, I'm unable to jump to f.png when pressing f (it does work with d.png / d). Interestingly, F works for f.png.

May 7 2018, 12:13 AM · Gwenview
huoni updated the diff for D11633: Disable View mode shortcuts outside of View mode.
  • Simplify patch to prevent duplicate calls
May 7 2018, 12:11 AM · Gwenview

May 3 2018

huoni committed R260:3f540441b394: Fix dark HUD icons when using light system theme (authored by huoni).
Fix dark HUD icons when using light system theme
May 3 2018, 11:39 PM
huoni closed D12595: Fix dark HUD icons when using light system theme.
May 3 2018, 11:39 PM · Gwenview
huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

I'm understanding this a bit better now. QIcon itself doesn't contain light/dark (palette) information. Only when you call QIcon::pixmap does it use the palette to generate either a light or dark icon (in this scenario). The palette used is the same as the QApplication palette, unless a custom palette is set with KIconLoader::setCustomPalette().

May 3 2018, 3:17 AM · Gwenview
huoni updated the diff for D12595: Fix dark HUD icons when using light system theme.
  • Fix HiDPI by changing how we generate the icon
May 3 2018, 3:08 AM · Gwenview

May 2 2018

huoni committed R302:b506a48214a0: Fix incorrect version in @since tag (authored by huoni).
Fix incorrect version in @since tag
May 2 2018, 12:11 AM
huoni closed D12650: Fix incorrect version in @since tag.
May 2 2018, 12:11 AM · Frameworks

May 1 2018

huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

One more thought: I case users already have a version >=5.39, we could already enable the fix for them on stable (with #ifdefs around the new API calls). OTOH, it's probably not worth the time figuring out how to do that with CMake…

May 1 2018, 11:36 PM · Gwenview
huoni requested review of D12650: Fix incorrect version in @since tag.
May 1 2018, 11:30 PM · Frameworks
huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

Seems Peter is right, both commits relating to these functions have v5.39.0-rc1 as their earliest tag.
Should I submit a patch that updates the @since ... comment?

Sure, fixing bugs in KDE's Frameworks is always appreciated ;)

In an ideal world, icons would change based on their parent widget's palette, rather than the entire applications palette...

Are you saying this is only a matter of finding the place where application should be changed to parent? Or perhaps just ask Marco what he thinks about that?

May 1 2018, 11:12 PM · Gwenview
huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

In fact, if this method of forcing light icons (and the corresponding KF5 version bump) is all good, I would prefer not landing this patch but instead convert the HUD to standard widgets (keeping the general HUD framework).
E.g. I would:

May 1 2018, 4:26 AM · Gwenview
huoni updated the summary of D12595: Fix dark HUD icons when using light system theme.
May 1 2018, 4:00 AM · Gwenview
huoni updated the diff for D12595: Fix dark HUD icons when using light system theme.
  • Change back to loadIcon and don't scale
  • Bump KF5 version to 5.39 to get setCustomPalette and resetPalette
May 1 2018, 3:58 AM · Gwenview
huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

Seems Peter is right, both commits relating to these functions have v5.39.0-rc1 as their earliest tag.
Should I submit a patch that updates the @since ... comment?

May 1 2018, 3:56 AM · Gwenview

Apr 29 2018

huoni added a comment to D12595: Fix dark HUD icons when using light system theme.

But this shouldn't be a problem for KDE 18.08.

…in case @huoni does not forget to bump the version.

Apr 29 2018, 10:29 PM · Gwenview
huoni updated the summary of D12595: Fix dark HUD icons when using light system theme.
Apr 29 2018, 7:11 AM · Gwenview
huoni added inline comments to D12595: Fix dark HUD icons when using light system theme.
Apr 29 2018, 7:10 AM · Gwenview
huoni updated the diff for D12595: Fix dark HUD icons when using light system theme.
  • Change back to QIcon
Apr 29 2018, 7:09 AM · Gwenview
huoni added inline comments to D12595: Fix dark HUD icons when using light system theme.
Apr 29 2018, 7:05 AM · Gwenview
huoni updated the summary of D12595: Fix dark HUD icons when using light system theme.
Apr 29 2018, 6:56 AM · Gwenview
huoni added a project to D12595: Fix dark HUD icons when using light system theme: Gwenview.
Apr 29 2018, 6:55 AM · Gwenview
huoni requested review of D12595: Fix dark HUD icons when using light system theme.
Apr 29 2018, 6:51 AM · Gwenview

Apr 28 2018

huoni added a comment to D12489: Remember width of sidebar.

Weirdly I'm now getting the cherry pick error when using arc patch. Something about dependent patches seems to confuse Arcanist.

From which branch did you issue arc patch from? For patches with dependencies, you'll probably have to git checkout master first, see my analysis in T8506. I guess you reviewed D12561 first and were still on this branch?

Apr 28 2018, 12:47 AM

Apr 27 2018

huoni accepted D12489: Remember width of sidebar.
Apr 27 2018, 10:07 PM
huoni added a comment to D12489: Remember width of sidebar.

Weirdly I'm now getting the cherry pick error when using arc patch. Something about dependent patches seems to confuse Arcanist.

Apr 27 2018, 10:07 PM
huoni accepted D12561: Set central splitter to non collapsible.

Easy review :)
Works as advertised, and code in apropriate place.

Apr 27 2018, 8:02 AM

Apr 26 2018

huoni added a comment to D12489: Remember width of sidebar.

Hadn't considered disabling the collapse behaviour entirely, and agree this is the best decision. The functionality doesn't make sense in this situation.

Apr 26 2018, 10:52 AM
huoni added a comment to D12489: Remember width of sidebar.

Well, I've had a play around, and found the following problems:

  1. If you switch to the Start Page before quitting, the sizes aren't saved.
  2. There's a pre-existing issue where collapsing the sidebar by dragging the splitter to the left doesn't toggle the sidebar off, and further toggling shows and hides a narrow sliver of the splitter. This patch makes this issue worse by saving the collapsed splitter state to config. I think the best solution to this would be to detect the collapse and 1) toggle the sidebar off, and 2) make sure the collapse splitter sizes aren't saved.
Apr 26 2018, 12:40 AM

Apr 25 2018

huoni committed R260:8f2997b91337: Ensure full screen info label shows at least two lines of text (authored by huoni).
Ensure full screen info label shows at least two lines of text
Apr 25 2018, 7:31 AM
huoni closed D12429: Ensure full screen info label shows at least two lines of text.
Apr 25 2018, 7:31 AM · Gwenview
huoni committed R260:14199f3bc2ed: Display document count labels in View, Browse, and Full Screen modes (authored by huoni).
Display document count labels in View, Browse, and Full Screen modes
Apr 25 2018, 7:25 AM
huoni closed D12301: Display document count labels in View, Browse, and Full Screen modes.
Apr 25 2018, 7:25 AM · Gwenview
huoni added inline comments to D12458: Overhaul image resize dialog.
Apr 25 2018, 7:14 AM
huoni accepted D12458: Overhaul image resize dialog.

Well the patch LGTM. Only one small quibble.
I suppose I will accept it!

Apr 25 2018, 7:07 AM
huoni committed R260:f88db58a516b: Change smooth zoom threshold from 200% to 400% (authored by huoni).
Change smooth zoom threshold from 200% to 400%
Apr 25 2018, 6:51 AM
huoni closed D12483: Change smooth zoom threshold from 200% to 400%.
Apr 25 2018, 6:51 AM · Gwenview
huoni added inline comments to D12301: Display document count labels in View, Browse, and Full Screen modes.
Apr 25 2018, 6:48 AM · Gwenview

Apr 24 2018

huoni added a comment to D12489: Remember width of sidebar.

@huoni Please review (including pressing the "Accept" button if both behaviour and code cannot be improved anymore ;) Let me know if you need tips on that.

Okay, I admit it: In the meantime I peeked at the patch a bit, and there might be a problem…

I haven't had time to look at this properly yet (and probably won't for another 24 hours), but looking at the code on my phone. Is the problem that loading the config always shows the sidebar, even if it was hidden?

If I hide the sidebar, it will stay hidden, so that's not the problem. Just wait until you have a chance to try the patch, but maybe you can also see it when looking critically at every line of the patch (i.e. typically you'd have to look for things that are either wrong, superfluous, or missing; and in particular look at edge cases).

Sorry for the cliffhanger, but we need more reviewers in Gwenview, so I just hope you are interested in learning how to review a bit ;)

Apr 24 2018, 11:52 PM
huoni added a comment to D12489: Remember width of sidebar.

@huoni Please review (including pressing the "Accept" button if both behaviour and code cannot be improved anymore ;) Let me know if you need tips on that.

Okay, I admit it: In the meantime I peeked at the patch a bit, and there might be a problem…

Apr 24 2018, 11:25 PM
huoni added inline comments to D12458: Overhaul image resize dialog.
Apr 24 2018, 11:09 PM
huoni added a comment to D12483: Change smooth zoom threshold from 200% to 400%.

LGTM. Looks just like the patch on my local branch, which I will now delete (apparently I'm bad at balancing reviews against submitting patches…).

Apr 24 2018, 8:11 AM · Gwenview
huoni added a comment to D12458: Overhaul image resize dialog.

Cherry-picking a patch which already has been applied only works when specifying --allow-empty, which Arcanist does not do ATM. We might have to look into whether we can change some defaults here, or if it can be changed upstream…

@muhlenpfordt For now, just use --skip-dependencies.

Apr 24 2018, 8:01 AM