D20197: Fix desktop link to file or directory

Nathaniel Graham noreply at phabricator.kde.org
Fri Apr 5 16:57:41 BST 2019


ngraham added a comment.


  Thanks, this works well now. The UI and behavior seem sane, and I've just got a few code changes requested:

INLINE COMMENTS

> kurlrequester.cpp:441
> +            QAction *fileAction = new QAction(QIcon::fromTheme(QStringLiteral("document-new")), i18n("File"));
> +            QAction *dirAction = new QAction(QIcon::fromTheme(QStringLiteral("document-open")), i18n("Directory"));
> +            dirOrFileMenu->addAction(fileAction);

This should probably be `folder-new` for visual consistency with the file icon you've chosen

> kurlrequester.cpp:445
> +
> +            QAction *dirOrFile = dirOrFileMenu->exec(m_parent->mapToGlobal(QPoint(m_parent->width(), m_parent->height())));
> +

This feels overly clever and fragile. The more typical approach would be to connect the actions' `triggered` signals to lambda functions that execute the code that you currently have in the if/else blocks below.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D20197

To: hoffmannrobert, #frameworks, ngraham
Cc: ngraham, kde-frameworks-devel, michaelh, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20190405/fd900872/attachment.html>


More information about the Kde-frameworks-devel mailing list