D18744: Add action in Edit menu to select the current page

Nathaniel Graham noreply at phabricator.kde.org
Wed Feb 13 18:55:44 GMT 2019


ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  Please test your changes. The new menu is not actually added to the Edit menu and the shortcut you chose conflicts with the print shortcut.

INLINE COMMENTS

> pageview.cpp:740
> +    QAction *selectCurrentPage = new QAction( this );
> +    selectCurrentPage->setIcon( QIcon::fromTheme( QStringLiteral( "edit-select-all" ) ) );
> +    ac->addAction( QStringLiteral("Select Current Page"), selectCurrentPage );

This icon is already used for the Select All action, and we don't want to re-use the same icon for two actions in the same menu. Let's remove it.

> pageview.cpp:741
> +    selectCurrentPage->setIcon( QIcon::fromTheme( QStringLiteral( "edit-select-all" ) ) );
> +    ac->addAction( QStringLiteral("Select Current Page"), selectCurrentPage );
> +    connect( selectCurrentPage, &QAction::triggered, this, &PageView::slotSelectPage );

The correct text here would be "Select all text on current page"

> pageview.cpp:743
> +    connect( selectCurrentPage, &QAction::triggered, this, &PageView::slotSelectPage );
> +    ac->setDefaultShortcut(selectCurrentPage, QKeySequence(Qt::CTRL + Qt::Key_P));
> +    addAction( selectCurrentPage );

Erm, that's the shortcut used for printing. Did you test this?

REPOSITORY
  R223 Okular

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

To: shubham, aacid, #vdg, ngraham
Cc: kde-doc-english, davidhurka, abetts, loh.tar, alexde, ngraham, okular-devel, gennad, tfella, skadinna, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20190213/8d6f98a6/attachment.html>


More information about the Okular-devel mailing list