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

[Nathaniel Graham]

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/kde-doc-english/attachments/20190213/8d6f98a6/attachment.html>