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