View Issue Details

IDProjectCategoryView StatusLast Update
0008745ScribusUndo/Redopublic2014-10-21 21:33
Reporterale Assigned ToChelen  
PrioritynormalSeverityfeatureReproducibilityN/A
Status closedResolutionwon't fix 
Product Version1.4 milestone 
Summary0008745: [PATCH NEEDS TESTING] undo and redo should focus to the concerned item
Descriptionundo 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 Informationsince 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...
Tagschelen, patch
Patch

Relationships

has duplicate 0005075 closedcbradney undo object deletion should make the object active 
related to 0009246 closedcbradney deactivate undo while doing actions which are not yet managed by the undo / redo manager 
related to 0004821 closedChelen page palette: deletion of masterpages and pages should only be possible after confirmation 
child of 0012500 acknowledged Metabug: Undo/Redo (1.5.x.svn) 

Activities

Chelen

2012-06-22 13:03

reporter   ~0028260

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.

Chelen

2012-06-27 20:07

reporter  

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());
focusUndo.diff (6,234 bytes)   

cezaryece

2012-06-28 07:04

updater   ~0028312

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.

Chelen

2012-07-25 13:11

reporter   ~0028620

Just to know, is it behave correctly when it is implemented?
If it is, I can continue and add focus for all undos/redos

ale

2012-07-26 20:47

manager   ~0028658

Where should I test it?

Chelen

2012-07-31 11:05

reporter   ~0028702

The patch is in this bug report.

Kunda

2014-10-21 21:33

updater   ~0034105

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

Issue History

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