D10932: [Okular] Option to reset forms

Albert Astals Cid noreply at phabricator.kde.org
Tue Mar 13 22:40:54 UTC 2018


aacid added a comment.


  Have you thought how this should interact with the undo/redo history stack?

INLINE COMMENTS

> resetformstest.cpp:182
> +{
> +    verifyTextForm( m_textLineForm );
> +}

what does this test do?

> part.cpp:3359
>      m_formsMessage->addAction( m_pageView->toggleFormsAction() );
> -
> +    m_formsMessage->addAction( m_pageView->resetFormsAction() );
>      // ensure history actions are in the correct state

Why did you add this?

> pageview.cpp:731
> +    d->aResetForms->setEnabled( false );
> +    resetFormWidgets( false );
> +

Why are you calling this?

> pageview.cpp:4333
> +    if(d->aResetForms)
> +        d->aResetForms->setText( i18n( "Reset Forms" ) );
> +    if( d->aResetForms && on)

why do you set the text every single time this is called  instead of on construction time?

> pageview.cpp:4433
> +{
> +    document->editFormButtons( pageNumber, formButtons, newButtonStates);
> +}

What's the point of all this one single line functions?

REPOSITORY
  R223 Okular

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

To: ahmadosama, #okular
Cc: ngraham, aacid, #okular, michaelweghorn
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20180313/cc632e66/attachment-0001.html>


More information about the Okular-devel mailing list