View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0008745 | Scribus | Undo/Redo | public | 2010-01-25 20:20 | 2014-10-21 21:33 |
Reporter | ale | Assigned To | Chelen | ||
Priority | normal | Severity | feature | Reproducibility | N/A |
Status | closed | Resolution | won't fix | ||
Product Version | 1.4 milestone | ||||
Summary | 0008745: [PATCH NEEDS TESTING] undo and redo should focus to the concerned item | ||||
Description | undo should change focus to the undone item, so I can see what was undone (because it makes more sense to see what was undone than to see what was not undone). | ||||
Additional Information | since not every action can be undone this is even more desirable to give some feedback when what is undone is not what was meant to be undone... | ||||
Tags | chelen, patch | ||||
Patch | |||||
has duplicate | 0005075 | closed | cbradney | undo object deletion should make the object active |
related to | 0009246 | closed | cbradney | deactivate undo while doing actions which are not yet managed by the undo / redo manager |
related to | 0004821 | closed | Chelen | page palette: deletion of masterpages and pages should only be possible after confirmation |
child of | 0012500 | acknowledged | Metabug: Undo/Redo (1.5.x.svn) |
|
This is on my git : gsoc12ux on branch UndoFeature . As it is an other branch, I have not add this in news undo/redo I did. |
|
focusUndo.diff (6,234 bytes)
diff --git a/Scribus/scribus/pageitem.cpp b/Scribus/scribus/pageitem.cpp index 8f6c499..b0d827c 100644 --- a/Scribus/scribus/pageitem.cpp +++ b/Scribus/scribus/pageitem.cpp @@ -4635,6 +4635,8 @@ void PageItem::restore(UndoState *state, bool isUndo) m_Doc->setMasterPageMode(oldMPMode); m_Doc->useRaster = useRasterBackup; m_Doc->SnapGuides = SnapGuidesBackup; + if(!doc()->view()->visibleCanvas().contains(xPos(),yPos())) + doc()->view()->setCanvasOrigin(xPos(),yPos()); } void PageItem::restorePasteInline(SimpleState *is, bool isUndo) diff --git a/Scribus/scribus/pageitem_latexframe.cpp b/Scribus/scribus/pageitem_latexframe.cpp index 4b43f1b..7be5669 100644 --- a/Scribus/scribus/pageitem_latexframe.cpp +++ b/Scribus/scribus/pageitem_latexframe.cpp @@ -508,6 +508,8 @@ void PageItem_LatexFrame::killProcess() void PageItem_LatexFrame::restore(UndoState *state, bool isUndo) { + if(!doc()->view()->visibleCanvas().contains(xPos(),yPos())) + doc()->view()->setCanvasOrigin(xPos(),yPos()); SimpleState *ss = dynamic_cast<SimpleState*>(state); if (!ss) { PageItem_ImageFrame::restore(state, isUndo); diff --git a/Scribus/scribus/scpage.cpp b/Scribus/scribus/scpage.cpp index 5254420..84e8c83 100644 --- a/Scribus/scribus/scpage.cpp +++ b/Scribus/scribus/scpage.cpp @@ -94,6 +94,8 @@ void ScPage::setPageName(const QString& newName) void ScPage::restore(UndoState* state, bool isUndo) { + if(doc()->currentPageNumber() != pageNr()) + doc()->view()->GotoPage(pageNr()); SimpleState* ss = dynamic_cast<SimpleState*>(state); if (ss) { @@ -311,6 +313,8 @@ void ScPage::restorePageItemCreation(ScItemState<PageItem*> *state, bool isUndo) return; int stateCode = state->transactionCode; PageItem *ite = state->getItem(); + if(!doc()->view()->visibleCanvas().contains(ite->xPos(),ite->yPos())) + doc()->view()->setCanvasOrigin(ite->xPos(),ite->yPos()); bool oldMPMode=m_Doc->masterPageMode(); if ((stateCode == 0) || (stateCode == 1)) { @@ -351,6 +355,8 @@ void ScPage::restorePageItemDeletion(ScItemState< QList<PageItem*> > *state, boo if (!state) return; QList<PageItem*> itemList = state->getItem(); + if(!doc()->view()->visibleCanvas().contains(itemList.at(0)->xPos(),itemList.at(0)->yPos())) + doc()->view()->setCanvasOrigin(itemList.at(0)->xPos(),itemList.at(0)->yPos()); int id = state->getInt("ITEMID"); int id2 = state->getInt("ID"); if (itemList.count() <= 0) @@ -404,10 +410,14 @@ void ScPage::restorePageItemConversion(ScItemState<std::pair<PageItem*, PageItem m_Doc->Items->replace(m_Doc->Items->indexOf(newItem), oldItem); oldItem->updatePolyClip(); m_Doc->AdjustItemSize(oldItem); + if(!doc()->view()->visibleCanvas().contains(oldItem->xPos(),oldItem->yPos())) + doc()->view()->setCanvasOrigin(oldItem->xPos(),oldItem->yPos()); } else { m_Doc->Items->replace(m_Doc->Items->indexOf(oldItem), newItem); + if(!doc()->view()->visibleCanvas().contains(newItem->xPos(),newItem->yPos())) + doc()->view()->setCanvasOrigin(newItem->xPos(),newItem->yPos()); } m_Doc->setMasterPageMode(oldMPMode); } diff --git a/Scribus/scribus/scribus.cpp b/Scribus/scribus/scribus.cpp index ecf6a82..863ee01 100644 --- a/Scribus/scribus/scribus.cpp +++ b/Scribus/scribus/scribus.cpp @@ -8917,9 +8917,13 @@ void ScribusMainWindow::restoreDeletePage(SimpleState *state, bool isUndo) UndoObject *tmp = undoManager->replaceObject(state->getUInt("DUMMY_ID"), doc->Pages->at(pagenr - 1)); delete tmp; + if(doc->currentPageNumber() != (pagenr -1)) + view->GotoPage(pagenr-1); } else { + if(doc->currentPageNumber() != (pagenr-1)) + view->GotoPage(pagenr-1); DummyUndoObject *duo = new DummyUndoObject(); uint id = static_cast<uint>(duo->getUId()); undoManager->replaceObject(doc->Pages->at(pagenr - 1)->getUId(), duo); @@ -8973,6 +8977,8 @@ void ScribusMainWindow::restoreAddPage(SimpleState *state, bool isUndo) } if (isUndo) { + if(doc->currentPageNumber() != delFrom) + view->GotoPage(delFrom); for (int i = delFrom - 1; i < delTo; ++i) { DummyUndoObject *duo = new DummyUndoObject(); @@ -9002,6 +9008,8 @@ void ScribusMainWindow::restoreAddPage(SimpleState *state, bool isUndo) state->getUInt(QString("Page%1").arg(i)), doc->Pages->at(i)); delete tmp; } + if(doc->currentPageNumber() != (delFrom-1)) + view->GotoPage(delFrom-1); } } diff --git a/Scribus/scribus/scribusdoc.cpp b/Scribus/scribus/scribusdoc.cpp index a957538..3d5ec14 100644 --- a/Scribus/scribus/scribusdoc.cpp +++ b/Scribus/scribus/scribusdoc.cpp @@ -2375,9 +2375,14 @@ void ScribusDoc::restoreMovePage(SimpleState *state, bool isUndo) } newPageTo=newPageFrom+pagesMoved; movePage(newPageFrom, newPageTo, newPageDest, newPosition); + if(currentPageNumber() != newPageDest) + view()->GotoPage(newPageDest); } - else + else { movePage(fromPage, toPage, dest, position); + if(currentPageNumber() != dest) + view()->GotoPage(dest); + } } @@ -4338,6 +4343,8 @@ void ScribusDoc::restoreMasterPageApplying(SimpleState *state, bool isUndo) applyMasterPage(oldName, pageNumber); else applyMasterPage(newName, pageNumber); + if(currentPageNumber() != pageNumber) + view()->GotoPage(pageNumber); } @@ -4367,7 +4374,8 @@ void ScribusDoc::restoreCopyPage(SimpleState *state, bool isUndo) if(m_ScMW->outlinePalette->isVisible()) m_ScMW->outlinePalette->BuildTree(); } - + if(currentPageNumber() != pnum) + view()->GotoPage(pnum); } diff --git a/Scribus/scribus/undogui.cpp b/Scribus/scribus/undogui.cpp index 49b6f87..4e34b2b 100644 --- a/Scribus/scribus/undogui.cpp +++ b/Scribus/scribus/undogui.cpp @@ -189,7 +189,7 @@ void UndoWidget::updateUndoActions() void UndoWidget::updateUndo(int steps) { - for (int i = 0; i < steps; ++i) + for (int i = 0; i < steps && undoItems.size() > 0; ++i) { redoItems.insert(redoItems.begin(), undoItems[0]); undoItems.erase(undoItems.begin()); @@ -200,7 +200,7 @@ void UndoWidget::updateUndo(int steps) void UndoWidget::updateRedo(int steps) { - for (int i = 0; i < steps; ++i) + for (int i = 0; i < steps && redoItems.size()>0; ++i) { undoItems.insert(undoItems.begin(), redoItems[0]); redoItems.erase(redoItems.begin()); |
|
Not working for text edit actions. In this case not only focus should be switched to frame where undo/redo makes any changes (now nothing changes) - at least cursor should be placed in changed text (at the end I guess) if not select undoing/redoing changes. |
|
Just to know, is it behave correctly when it is implemented? If it is, I can continue and add focus for all undos/redos |
|
Where should I test it? |
|
The patch is in this bug report. |
|
Per Craig on IRC: i wont be applying it my reasoning: you might undo things that you have not actually touched, so why would you want them to be selected its just moving to and selecting an item as someone goes through the undo list eg.. draw a square, then a circle, then a rectangle... resize the rectangle.. undo twice, you'd end up with the circle selected but.. if there's cases where we select 10 items, make a change then which one would be selected via the undo process? anyway.. i think its about presuming a workflow when we cannot, same as auto selecting the tab on the PP.. just not reasonable |
Date Modified | Username | Field | Change |
---|---|---|---|
2010-01-25 20:20 | ale | New Issue | |
2010-07-16 09:07 | ale | Relationship added | related to 0009246 |
2012-06-20 21:36 | ale | Relationship added | related to 0004821 |
2012-06-22 13:03 | Chelen | Note Added: 0028260 | |
2012-06-22 13:03 | Chelen | Tag Attached: patch | |
2012-06-22 13:03 | Chelen | Tag Attached: chelen | |
2012-06-22 13:03 | Chelen | Tag Attached: fixed | |
2012-06-23 15:12 | cbradney | Relationship added | has duplicate 0005075 |
2012-06-27 20:07 | Chelen | File Added: focusUndo.diff | |
2012-06-28 07:04 | cezaryece | Note Added: 0028312 | |
2012-07-25 13:11 | Chelen | Note Added: 0028620 | |
2012-07-25 13:12 | Chelen | Assigned To | => Chelen |
2012-07-25 13:12 | Chelen | Status | new => assigned |
2012-07-26 20:47 | ale | Note Added: 0028658 | |
2012-07-31 11:05 | Chelen | Note Added: 0028702 | |
2014-07-13 19:40 | Kunda | Relationship added | child of 0012500 |
2014-07-13 19:41 | Kunda | Summary | undo and redo should focus to the concerned item => [PATCH] undo and redo should focus to the concerned item |
2014-10-13 15:59 | Kunda | Summary | [PATCH] undo and redo should focus to the concerned item => [PATCH NEEDS TESTING] undo and redo should focus to the concerned item |
2014-10-13 15:59 | Kunda | Tag Detached: fixed | |
2014-10-21 21:33 | Kunda | Note Added: 0034105 | |
2014-10-21 21:33 | Kunda | Status | assigned => closed |
2014-10-21 21:33 | Kunda | Resolution | open => won't fix |