D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

This revision was automatically updated to reflect the committed changes. Closed by commit R223:d50c06df2590: Add refresh widgets if underlying field changes (authored by aheinecke, committed by aacid). REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10048?vs=27

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid accepted this revision. This revision is now accepted and ready to land. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10048 To: aheinecke, #okular, aacid Cc: aacid, michaelweghorn, ngraham

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid added inline comments. INLINE COMMENTS > aheinecke wrote in document.cpp:1139 > Sorry I can't follow you here. > > If an action is processed from somewhere else the scripter won't have an > event set and the event pointer in the scripter is null. > > If the processAction is triggered her

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke added inline comments. INLINE COMMENTS > aacid wrote in document.cpp:1139 > This is problematic since it will leave a dangling event pointer for any > Action::Script action executed through Document::processAction that doesn't > come from this function. Sorry I can't follow you here.

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid added inline comments. INLINE COMMENTS > document.cpp:1139 > +m_scripter = new Scripter( this ); > +m_scripter->setEvent( event.get() ); > +// The value maybe changed in javascript so > save

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke updated this revision to Diff 27266. aheinecke added a comment. Update to build event with non const page ref after change in D10073 REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10048?vs=27257&id=27266 REVIS

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke marked 6 inline comments as done. aheinecke added a comment. So I only changed the version to 1.4 here. REPOSITORY R223 Okular REVISION DETAIL https://phabricator.kde.org/D10048 To: aheinecke, #okular Cc: aacid, michaelweghorn, ngraham, simgunz

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke updated this revision to Diff 27257. aheinecke added a comment. Changed comment about version which added the new signal. REPOSITORY R223 Okular CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D10048?vs=26761&id=27257 REVISION DETAIL https://phabricator.kde.org/D10048

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid added inline comments. INLINE COMMENTS > aheinecke wrote in document.cpp:1153 > Sure this would be nice but as I understand it this would mean making > FormFieldText (or the general FormField) a QObject. As this is public API i > shied away from such a solution. > > Should I change the p

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke added a comment. In D10048#205211 , @aacid wrote: > Haven't had time to try myself, but how all this works with undo/redo? Does stuff break or does it work nice? Ah, no. Undo / Redo does not trigger a new evaluation of calculate

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid added a comment. Haven't had time to try myself, but how all this works with undo/redo? Does stuff break or does it work nice? INLINE COMMENTS > document.h:1200 > + * according widget should be updated. > + * @since 1.3.70 > + */ make this 1.4 which will be the

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke added inline comments. INLINE COMMENTS > aacid wrote in document.cpp:1153 > I was thinking, can't we do setText emit the signal? so we don't really need > to worry about forgetting to emit refreshFormWidget in case we end up > implementing another function or something that does chang

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aacid added inline comments. INLINE COMMENTS > document.cpp:1153 > +fft->setText( newVal ); > +emit m_parent->refreshFormWidget( fft ); > +pageNeedsRefresh = true; I was thinking, can't we

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke added a comment. In https://phabricator.kde.org/D10048#202922, @aheinecke wrote: > Thanks. Still with current master this does not work for me as in the video. When I zoom in and out it works but not automatically through the refresh-pixmaps in the field setter. With current ma

D10048: [PATCH 2/4] Add refresh widgets if underlying field changes

aheinecke updated this revision to Diff 26761. aheinecke retitled this revision from " [PATCH 2/4] Communicate calculate text change to formwidgets" to "[PATCH 2/4] Add refresh widgets if underlying field changes". aheinecke edited the summary of this revision. aheinecke added a comment. Prop