ngraham (Nathaniel Graham)
User

Projects (12)

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Tuesday

  • Clear sailing ahead.

User Details

User Since
Apr 15 2017, 7:18 PM (45 w, 10 h)
Availability
Available

Recent Activity

Today

ngraham added a comment to D10812: Fix non-working "-n" or "--nonotify" switch.

Thanks for the patch! Reading the --help text, it seems like this is an option designed to be run in conjunction with --background` mode:

Sun, Feb 25, 3:12 AM
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

Maybe send an email to kde-devel@kde.org?

Sun, Feb 25, 2:51 AM · Dolphin

Yesterday

ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

Yes, that would be in a separate diff. If you'd like, implement the feature in Dolphin for now, and then we can remove it when your KIO patch lands. Otherwise we'll have to delay this by a few weeks or more, waiting for whatever KDE Frameworks version your KIO patch lands in.

Sat, Feb 24, 11:25 PM · Dolphin
ngraham added a comment to T8071: Editing after renaming a file does not update preview and saves as previous filename.

Cool!

Sat, Feb 24, 11:23 PM · Gwenview
ngraham added a comment to T8071: Editing after renaming a file does not update preview and saves as previous filename.

We still use Bugzilla to track bugs and small tasks; see https://community.kde.org/Infrastructure/Phabricator

Sat, Feb 24, 11:17 PM · Gwenview
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

It would be better to show button only if trash is not empty. KIO (https://api.kde.org/frameworks/kio/html/namespaceKIO.html) doesn't have isTrashEmpty function. Now Dolphin uses special code to check it. I propose to add it there. Can you point me at KIO repo?

Sat, Feb 24, 11:12 PM · Dolphin
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

No need for a space between FEATURE and the :

Sat, Feb 24, 11:10 PM · Dolphin
ngraham updated subscribers of D10770: Icon fixes to choose more appropriate icons in some areas.

In what way does it break Oxygen? What's missing?

Sat, Feb 24, 11:03 PM · Gwenview
ngraham added inline comments to D10804: Show "Empty trash" button inside trash directory.
Sat, Feb 24, 10:52 PM · Dolphin
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

Make sure it compiles before pushing, please. Now it no longer does for me:

Sat, Feb 24, 10:49 PM · Dolphin
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

There is 3 places that is responsible for emptying trash. 2 of them use code from trash/dolphintrash.cpp. Third place is https://github.com/KDE/dolphin/blob/master/src/panels/places/placespanel.cpp#L458

The code differs a little bit and I don't understand why. What should we do with that?

Sat, Feb 24, 10:40 PM · Dolphin
ngraham requested changes to D10804: Show "Empty trash" button inside trash directory.
Sat, Feb 24, 10:34 PM · Dolphin
ngraham added a comment to D10804: Show "Empty trash" button inside trash directory.

Thanks for the patch! Couple of notes:

Sat, Feb 24, 10:34 PM · Dolphin
ngraham committed R433:01876011f5ed: Merge branch 'Applications/17.12' (authored by ngraham).
Merge branch 'Applications/17.12'
Sat, Feb 24, 2:52 PM
ngraham added a comment to D10794: Fix blurry icons in HiDPI.

Shipped!

Sat, Feb 24, 2:51 PM
ngraham committed R433:bc64bce6050e: Fix blurry icons in HiDPI (authored by ngraham).
Fix blurry icons in HiDPI
Sat, Feb 24, 2:51 PM
ngraham closed D10794: Fix blurry icons in HiDPI.
Sat, Feb 24, 2:51 PM
ngraham updated the summary of D10794: Fix blurry icons in HiDPI.
Sat, Feb 24, 2:47 PM
ngraham requested review of D10794: Fix blurry icons in HiDPI.
Sat, Feb 24, 2:45 PM
ngraham added a reviewer for D10792: Raise annotation window when clicking on annotation: aacid.

Can confirm, this definitely improves things with overlapping notes.

Sat, Feb 24, 2:23 PM · Okular
ngraham committed R245:e94ba0619477: [UDisks] Fix auto-mount regression (authored by dkhlestkov).
[UDisks] Fix auto-mount regression
Sat, Feb 24, 2:16 PM
ngraham closed D10671: [UDisks] Fix auto-mount regression.
Sat, Feb 24, 2:16 PM · Frameworks
ngraham added a comment to D10671: [UDisks] Fix auto-mount regression.

Looks like not. Landing it.

Sat, Feb 24, 2:16 PM · Frameworks
ngraham added a comment to D10719: Highlighting for OpenSCAD.

Oops, looks like your last update replaced the original change.

Sat, Feb 24, 2:12 PM · Frameworks
ngraham added a reviewer for D10792: Raise annotation window when clicking on annotation: Okular.
Sat, Feb 24, 1:52 PM · Okular
ngraham added a comment to D10791: Fixed the Wikimedia Commons Picture of the Day provider.

No need to abandon the revision, you can just mark it as "Changes Planned".

Sat, Feb 24, 1:44 PM · Plasma

Fri, Feb 23

ngraham added a comment to D10770: Icon fixes to choose more appropriate icons in some areas.

Awesome patch; this is gonna look so good soon.

Fri, Feb 23, 11:06 PM · Gwenview
ngraham accepted D10763: Fix background color for image view toolbar (crop/red-eye).

Much better! Hope you don't mind that I re-arranged your screenshots; it's easier to see the differences between befores and afters by switching between them rapidly using the left and right arrow keys if they're adjacent.

Fri, Feb 23, 10:21 PM
ngraham updated the summary of D10763: Fix background color for image view toolbar (crop/red-eye).
Fri, Feb 23, 10:20 PM
ngraham accepted D10778: balooctl: Remove checkDb option.

+1, get rid of that thing!

Fri, Feb 23, 8:34 PM · Baloo, Frameworks
ngraham added a comment to T8033: Collapse "Active Window" and "Window under Cursor" modes.

In fact, Spectacle already has "click on a window to select it" mode.

Fri, Feb 23, 6:14 PM · Spectacle
ngraham added a comment to D10763: Fix background color for image view toolbar (crop/red-eye).

Hmm, in the After Normal screenshot, I don't like how there's now no line or other visual separation between the footer and the new toolbar. The two elements seem to blend into one another. I kind of liked it better before, IMHO.

Fri, Feb 23, 5:51 PM
ngraham added a comment to D8958: Fix unintentional breadcrumb menu item activation.

@aleksejshilin doesn't have commit access, so would you like to land this, @dfaure?

Fri, Feb 23, 5:42 PM · Frameworks
ngraham added a comment to D10455: Add RTL support for search, copy & paste in pdf.

@fahadalsaidi, I recently migrated an older patch of yours from Reviewboard to Phabricator (D10298), but I just closed it in favor of this one. If D10298 is still relevant even with this patch, please commandeer and re-open it. Thanks!

Fri, Feb 23, 5:03 PM · Okular
ngraham abandoned D10298: Fix searching in RTL PDFs.

Thanks for the test! However, the original author of this patch recent re-appeared here on Phabricator and submitted a better one: D10455: Add RTL support for search, copy & paste in pdf.

Fri, Feb 23, 5:01 PM · Okular
ngraham closed T7943: Icons in Ubiquity slideshow cut off on the bottom as Resolved.
Fri, Feb 23, 3:02 PM · Kubuntu
ngraham added a comment to T8061: Patch NetworkManager to avoid double wifi password pop-up issue.

OK cool! Feel free to close this if the results of that discussion prove fruitful.

Fri, Feb 23, 2:57 PM · Kubuntu
ngraham created T8061: Patch NetworkManager to avoid double wifi password pop-up issue.
Fri, Feb 23, 2:48 PM · Kubuntu
ngraham added a comment to T8057: Remove our Desktop Toolbox positioning patch for Plasma 5.13.0.

No worries, you're all good!

Fri, Feb 23, 2:42 PM · Kubuntu
ngraham added a comment to T8057: Remove our Desktop Toolbox positioning patch for Plasma 5.13.0.

Up to you; I don't have strong feelings on the matter.

Fri, Feb 23, 2:38 PM · Kubuntu
ngraham added a comment to D10671: [UDisks] Fix auto-mount regression.

Do any Frameworks folks want to chime in before I land this?

Fri, Feb 23, 2:37 PM · Frameworks
ngraham added a comment to D9414: Implement OSD to select action when unknown monitors is connected.

Is the screenshot in the Summary section still accurate? I'd like to feature this in the coming weekend's Usability & Productivity blog post (assuming it lands today or tomorrow; if not, next week's post), and people love looking at screenshots.

Fri, Feb 23, 2:05 PM · Plasma
ngraham created T8057: Remove our Desktop Toolbox positioning patch for Plasma 5.13.0.
Fri, Feb 23, 2:02 PM · Kubuntu

Thu, Feb 22

ngraham committed R134:33329175b58e: Change "Last Release" to "What's New" (authored by ngraham).
Change "Last Release" to "What's New"
Thu, Feb 22, 11:06 PM
ngraham committed R134:b05b2f540874: Merge branch 'Plasma/5.12' (authored by ngraham).
Merge branch 'Plasma/5.12'
Thu, Feb 22, 11:03 PM
ngraham committed R134:bfa25177805f: Reduce sidebar width (authored by ngraham).
Reduce sidebar width
Thu, Feb 22, 11:03 PM
ngraham closed D10756: Reduce sidebar width.
Thu, Feb 22, 11:02 PM · Plasma
ngraham updated the test plan for D10756: Reduce sidebar width.
Thu, Feb 22, 10:51 PM · Plasma
ngraham updated the test plan for D10756: Reduce sidebar width.
Thu, Feb 22, 10:44 PM · Plasma
ngraham updated the diff for D10756: Reduce sidebar width.

Use the right branch

Thu, Feb 22, 10:39 PM · Plasma
ngraham updated the summary of D10756: Reduce sidebar width.
Thu, Feb 22, 10:38 PM · Plasma
ngraham requested review of D10756: Reduce sidebar width.
Thu, Feb 22, 10:29 PM · Plasma
ngraham added a comment to D10709: Save windows screenshots with filename containing title.

@rominf Thanks for the updates, I'll look at the code later. Meanwhile you could improve your summary and your test plan a bit. The bug is only for reference, i.e. readers of git log should not be expected to open it to be able to understand what the change is about. See also https://chris.beams.io/posts/git-commit/.

@ngraham What would be your preference: Adding "Fullscreen/Desktop/Region" when no window title is available, or simply omitting it? Or add both options via %W and %w, at the expense of yet another entry and a confusing description?

Thu, Feb 22, 9:55 PM · Spectacle
ngraham accepted D10754: Ported page about elevation and shadows to rst.
Thu, Feb 22, 8:04 PM · KDE Human Interface Guidelines
ngraham added a comment to D10753: Provide the source selection as a contextual action.

OK, the i18n error is gone now. But do we really need that "also available in XXXX" text at the end of the Description, anyway? Apparently nobody reads it. Maybe we should focus on polishing the main source chooser UI and get rid of that little vestigial one.

Thu, Feb 22, 7:52 PM · Plasma
ngraham requested changes to D10753: Provide the source selection as a contextual action.

I see what you're going for here, but the "Sources" menu you're trying to add still doesn't actually work, same as the Sort menu. Is there an open Kirigami patch that this depends upon?

Thu, Feb 22, 7:46 PM · Plasma
ngraham committed R260:ac5c5a25216a: Merge branch 'Applications/17.12' (authored by ngraham).
Merge branch 'Applications/17.12'
Thu, Feb 22, 7:21 PM
ngraham committed R260:efaebe290f1b: Add icons for context and sidebar menu (authored by acrouthamel).
Add icons for context and sidebar menu
Thu, Feb 22, 7:20 PM
ngraham closed D10726: Add icons for context and sidebar menu.
Thu, Feb 22, 7:20 PM · Gwenview
ngraham added a comment to D10475: Make it possible to show the title despite having ctx actions.

The title is still the primary focus for this header UI, which means (for RTL languages at least), it needs to be left or center aligned. This means the actions have to be on the right, to make sure your eyes don't see them before the title.

Thu, Feb 22, 6:29 PM · Kirigami
ngraham added a comment to D10726: Add icons for context and sidebar menu.

One more thing I just noticed, if you want bonus points:

use edit-delete-symbolic for the red Trash icons, so their lines have the same weight as the black/white Trash icons.

Interesting. Looking at it that way, we'd also have to use document-open-folder instead of folder-open-symbolic, or perhaps I'm just confused now!?

Thu, Feb 22, 6:16 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

One more thing I just noticed, if you want bonus points:

use edit-delete-symbolic for the red Trash icons, so their lines have the same weight as the black/white Trash icons.

Ok 1 sec. It looks like that icon is built-in, so I believe I can override it.

In that case, we might instead need to change a KStandardAction somewhere...

Would you prefer me to not override here then and we'll just fix KStandardAction?

Thu, Feb 22, 6:10 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

One more thing I just noticed, if you want bonus points:

use edit-delete-symbolic for the red Trash icons, so their lines have the same weight as the black/white Trash icons.

Ok 1 sec. It looks like that icon is built-in, so I believe I can override it.

Thu, Feb 22, 6:07 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

@acrouthamel, if you'd like to land this on stable, then you'll need to re-base it on the stable branch. See https://community.kde.org/Infrastructure/Phabricator#Landing_on_the_.22Stable_branch.22

Thu, Feb 22, 6:07 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

One more thing I just noticed, if you want bonus points:

Thu, Feb 22, 6:00 PM · Gwenview
ngraham accepted D10726: Add icons for context and sidebar menu.

Now that looks professional! Let's ship it!

Thu, Feb 22, 5:57 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

Any ideas for folder-open?

folder-open-symbolic works too. Would that be even better?

Thu, Feb 22, 5:52 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

Hmm, not sure I agree. The -symbolic icons exist specifically for this purpose: to always show a "symbolic" version. The non-symbolic version happens to look identical at a small size, but when scaled up,

Thu, Feb 22, 5:44 PM · Gwenview
ngraham added a comment to D10726: Add icons for context and sidebar menu.

I think there's one final bit of polish we can do here. In the "after" screenshots, most icons use the "symbolic" line-art stylr with Breeze, but a few show colored folders when run in HiDPI mode, which your screenshots depict. Perhaps we should take the opportunity to fix this, too. It's especially odd for the Trash icon.

Thu, Feb 22, 5:34 PM · Gwenview
ngraham committed R166:d8c78b293411: Remove "Remember last used Save mode" checkbox (authored by rominf).
Remove "Remember last used Save mode" checkbox
Thu, Feb 22, 5:08 PM
ngraham closed D10711: Remove "Remember last used Save mode" checkbox.
Thu, Feb 22, 5:08 PM · Spectacle
ngraham accepted D10711: Remove "Remember last used Save mode" checkbox.

Looks perfect now! Landing it on master.

Thu, Feb 22, 5:05 PM · Spectacle
ngraham added a comment to D10671: [UDisks] Fix auto-mount regression.

Can this land?

Thu, Feb 22, 3:12 PM · Frameworks
ngraham added a comment to D10719: Highlighting for OpenSCAD.

In case anyone wants a more complex test file, I'm attaching one (written by me, MIT license):

Thu, Feb 22, 2:40 PM · Frameworks
ngraham added a comment to T7964: Wayland Mouse KCM.

Related bug: https://bugs.kde.org/show_bug.cgi?id=350688

Thu, Feb 22, 2:27 PM · Plasma on Wayland, Plasma (Plasma 5.13)
ngraham added a comment to D10726: Add icons for context and sidebar menu.

Yes, these can't be Frameworks changes via KStandardAction because they're all Gwenview-specific. There is still one missing icon in context menus: Edit Tags. @acrouthamel, if you want to add that one too, that would be nice. tag-new seems like an appropriate icon.

Thu, Feb 22, 12:14 AM · Gwenview
ngraham committed R134:887915abc904: Merge branch 'Plasma/5.12' (authored by ngraham).
Merge branch 'Plasma/5.12'
Thu, Feb 22, 12:08 AM
ngraham accepted D10726: Add icons for context and sidebar menu.

Wonderful. Works great! Those missing icons always bugged me, too.

Thu, Feb 22, 12:08 AM · Gwenview
ngraham committed R134:4e561fe2be3f: Fix ApplicationScreenshots shadow for Breeze Dark. (authored by akrutzler).
Fix ApplicationScreenshots shadow for Breeze Dark.
Thu, Feb 22, 12:08 AM
ngraham closed D10701: Fix ApplicationScreenshots shadow for Breeze Dark..
Thu, Feb 22, 12:07 AM · Plasma

Wed, Feb 21

ngraham added a comment to D10701: Fix ApplicationScreenshots shadow for Breeze Dark..

Thanks! I'll land this later tonight, if Alex doesn't beat me to it.

Wed, Feb 21, 11:10 PM · Plasma
ngraham added a comment to D10701: Fix ApplicationScreenshots shadow for Breeze Dark..

Since this is a small bugfix, I'd like to land it on the Stable branch (Plasma 5.12). However, I can't easily do this using arc because you did the work on master without a branch (in the future, see https://community.kde.org/Infrastructure/Phabricator#Workflow). Can you put it on a branch and re-base this patch on Plasma 5.12 branch? Here's how to do that:

Wed, Feb 21, 5:22 PM · Plasma
ngraham added a comment to D10711: Remove "Remember last used Save mode" checkbox.

Much better! Still needs a few things:

  • Actual testing steps listed in the Test Plan section, not just a listing of your test config
  • We can still remove more code. See D10711#210888. We can get rid of every usage of UseDynamicSaveButton and a checkbox in generalOptionsPage.h, and finally simplify KSMainWindow::saveButtonMode()
Wed, Feb 21, 5:07 PM · Spectacle
ngraham added a comment to D10702: Always use a job to delete files to avoid freezing process waiting on IO.

Thanks! Still gotta remove "BUG: 390748" from the title, though.

Wed, Feb 21, 4:02 PM · Frameworks
ngraham awarded D10719: Highlighting for OpenSCAD a Mountain of Wealth token.
Wed, Feb 21, 3:59 PM · Frameworks
ngraham added a reviewer for D10711: Remove "Remember last used Save mode" checkbox: Spectacle.

Also, please mention your testing process in the Test Plan section, and be sure to add the Spectacle group in the future.

Wed, Feb 21, 3:35 PM · Spectacle
ngraham requested changes to D10711: Remove "Remember last used Save mode" checkbox.

Migrating my comment from D10718, since I saw that one before this one. We'll use your patch, since you got in first. :)

Wed, Feb 21, 3:31 PM · Spectacle
ngraham accepted D10701: Fix ApplicationScreenshots shadow for Breeze Dark..
Wed, Feb 21, 3:23 PM · Plasma
ngraham requested changes to D10718: Remove "Remember last used Save mode" checkbox in Configure window.

Thanks! Great second patch. There's even more code clean-up we can do here, though. For example:

Wed, Feb 21, 3:21 PM · Spectacle
ngraham added a comment to D10709: Save windows screenshots with filename containing title.

Thanks for the patch! Instead of "Implementation of https://bugs.kde.org/show_bug.cgi?id=378463", could you change that to "BUG: 378463"? See https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

Wed, Feb 21, 2:44 PM · Spectacle
ngraham added a reviewer for D10709: Save windows screenshots with filename containing title: Spectacle.
Wed, Feb 21, 2:43 PM · Spectacle
ngraham added a comment to D10672: Set screen bounds for active window grab when compositor is disabled.

Use of Latte Dock, maybe? No idea.

Wed, Feb 21, 2:25 PM · Spectacle
ngraham added a comment to D10672: Set screen bounds for active window grab when compositor is disabled.

Works for me:

Looks like you used Window Under Cursor, but here we are all about Active Window. Could you try again, please?

For me, with 1.5x scaling I still get a null image as soon as Kate touches the bottom edge of the screen (if it only touches the bottom panel it's fine, touching left/top/right is good too).

Wed, Feb 21, 2:03 PM · Spectacle
ngraham accepted D10682: Prevent occasional empty Save button when upgrading from earlier version.

Thanks @rkflx, great catch. Can reproduce the issue, and your patch fixes it. Nicely done.

Wed, Feb 21, 5:15 AM
ngraham added a comment to D10673: Fix capturing QDialog that included the whole desktop area.

Please open a new task for that so we can figure out what it is Spectacle currently supports, what is broken and needs to be fixed and how we can present this better to the user.

Wed, Feb 21, 5:13 AM · Spectacle
ngraham added a comment to D10672: Set screen bounds for active window grab when compositor is disabled.

Works for me:

Wed, Feb 21, 5:06 AM · Spectacle
ngraham committed R166:db2231ae618a: Fix capturing QDialog that included the whole desktop area (authored by anemeth).
Fix capturing QDialog that included the whole desktop area
Wed, Feb 21, 4:54 AM
ngraham closed D10673: Fix capturing QDialog that included the whole desktop area.
Wed, Feb 21, 4:54 AM · Spectacle
ngraham created T8033: Collapse "Active Window" and "Window under Cursor" modes.
Wed, Feb 21, 4:53 AM · Spectacle

Tue, Feb 20

ngraham added a comment to D10702: Always use a job to delete files to avoid freezing process waiting on IO.

Thanks for the patch! Please remove "bug 390748" from the title and instead add "BUG: 390748" to its own line in the Summary section. See https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch

Tue, Feb 20, 11:03 PM · Frameworks