D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid commandeered this revision.
aacid edited reviewers, added: ahmadosama; removed: aacid.

REPOSITORY
  R223 Okular

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

To: aacid, ahmadosama
Cc: okular-devel, aacid, ngraham, #okular

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid abandoned this revision.

REPOSITORY
  R223 Okular

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

To: aacid, ahmadosama
Cc: okular-devel, aacid, ngraham, #okular

Re: D10504: Bug 288042 - Option to reset forms

[Ahmad Osama]

yes I am going to cancel this revision.

On Wed, Mar 14, 2018 at 12:49 AM, Albert Astals Cid <
nore...@phabricator.kde.org> wrote:

> aacid added a comment. View Revision 
>
> So will you cancel this one in favor of the other one?
>
> *REPOSITORY*
> R223 Okular
>
> *REVISION DETAIL*
> https://phabricator.kde.org/D10504
>
> *To: *ahmadosama, aacid
> *Cc: *aacid, ngraham, Okular, michaelweghorn
>

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid added a comment.


  So will you cancel this one in favor of the other one?

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Ahmad Osama]

ahmadosama added a comment.


  I implemented the reset option using Okular::FormFields inspired by the 
comments here:
  https://git.reviewboard.kde.org/r/130057/
  
  I submitted another patch for this new implementation here:
  https://phabricator.kde.org/D10932
  
  I am currently working on the creation of the auto tests, I think using this 
implementation the auto test will be similar to editformstext.cpp and I will 
upload the auto test for review once it is done.

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

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


  Ok, ask if you have questions :)

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Ahmad Osama]

ahmadosama added a comment.


  I have not created an auto test before, I am currently going through current 
auto tests to understand how it works, then I will try creating an auto test 
for this bug.

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid added a comment.


  Do you plan to work on the autotest?

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn

D10504: Bug 288042 - Option to reset forms

[Ahmad Osama]

ahmadosama updated this revision to Diff 27427.
ahmadosama added a comment.


  Removing unnecessary comments, add clear widget function for other widgets.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10504?vs=27143&id=27427

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

AFFECTED FILES
  part.cpp
  ui/formwidgets.cpp
  ui/formwidgets.h
  ui/pageview.cpp
  ui/pageview.h

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn, simgunz

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid requested changes to this revision.
This revision now requires changes to proceed.

REPOSITORY
  R223 Okular

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

To: ahmadosama, aacid
Cc: aacid, ngraham, #okular, michaelweghorn, simgunz

D10504: Bug 288042 - Option to reset forms

[Albert Astals Cid]

aacid added a comment.


  Same as I asked Gilbert in reviewboard.
  
  Do you think you could do an autotest using formSamples.pdf ?

INLINE COMMENTS

> part.cpp:1548
>  {
> +/* original
>  m_formsMessage->setText( i18n( "This document has forms. Click on 
> the button to interact with them, or use View -> Show Forms." ) );

This is not how you do coding, you don't leave the old code commented around.

> formwidgets.cpp:552
> +// TODO:
> +setText("");
> +}

this is wrong, you want QString()

> formwidgets.h:160
>  
> +//ChangeHere
> +virtual void clearWidgetContents();

Remove all those markers, we can see the changes.

REPOSITORY
  R223 Okular

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

To: ahmadosama
Cc: aacid, ngraham, #okular, michaelweghorn, simgunz

D10504: Bug 288042 - Option to reset forms

[Ahmad Osama]

ahmadosama created this revision.
ahmadosama added a project: Okular.
Restricted Application added a subscriber: Okular.
ahmadosama requested review of this revision.

REVISION SUMMARY
  These changes are done to add a button to Reset pdf forms when clicked.
  
  Changes are to the following files:
  
  - part.cpp: Just changing the text displayed to the user when the document 
has forms
  - pageview.cpp/pageview.h: add an action to reset forms just like the Show 
Form/Hide form (aToggleForm QAction)
  - formwidget.cpp/formwidger.h: add a virtual function 'clearWidgetContents()' 
to the parent class (FormWidgetIface) that clears the contents of the widget, 
then overriding this function in its children to clear the contents differently 
in each widget, ex. clearing the FormLineEditClass (that iherits from 
QLineEdit) is done by setting text to an empty string ("").
  
  - the clearWidgetContents() is called when the QAction associated with it is 
triggered.
  
  *the clearWidgetContent() is added to the FormLineEdit, radioButton, 
checkBoxEdit and KTextEdit

REPOSITORY
  R223 Okular

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

AFFECTED FILES
  part.cpp
  ui/formwidgets.cpp
  ui/formwidgets.h
  ui/pageview.cpp
  ui/pageview.h

To: ahmadosama
Cc: #okular, michaelweghorn, ngraham, simgunz, aacid