Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

changes to annotations mark the project dirty (fixes #7586) #2506

Merged
merged 1 commit into from
Nov 30, 2015

Conversation

SebDieBln
Copy link
Contributor

Changes to annotations (adding, editing, resizing, moving, deleting) now mark the project dirty. So these changes can't just vanish unnoticed when closing the project.

When testing please keep in mind Redmine # 13887, e.g. make sure to have at least one layer in the project.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 29, 2015

How are they saved to the project eventually?
I guess I am looking for some "model" (as in MVC) where the data is managed. Could this logic be added there instead?

@SebDieBln
Copy link
Contributor Author

There appears to be no model. Annotations are derived from QgsMapCanvasItem and they are added directly to the canvas upon creation.

Annotations are saved by QgisApp::writeAnnotationItemsToProject( QDomDocument& doc ). It finds the annotations by trying to cast every item on the canvas to QgsAnnotationItem.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 30, 2015

What do you think about adding it to the QgsAnnotationItem code instead of the map tool?
I think it's safer if it's changed from somewhere else than the maptool.

@SebDieBln
Copy link
Contributor Author

That actually was my first approach. But then every project containing annotations was dirty, because during the loading of a project QgsAnnotationItemss are constructed, too.

I agree that annotations might be changed from a plugin or somewhere else and we want to catch that, too. I will rework this PR and come back to you then.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 30, 2015

If you have any ideas left, ok, go for it.
But I won't mind merging this approach instead. Your implementation seems to be an appropriate workaround for the situation.

@SebDieBln
Copy link
Contributor Author

Well, it was a dead end 😞 So please merge this as it is.

Maybe we should take a mental note to rewrite the annotation code when going for the next major release?

@nyalldawson
Copy link
Collaborator

@SebDieBln I'd also like to see this. One issue is that it's locked up in GUI and not accessible from CORE... which causes problems when trying to use annotations in composer maps.

@m-kuhn
Copy link
Member

m-kuhn commented Nov 30, 2015

Add a

// TODO QGIS3

note, that's what I'll grep for when it's time :)

this includes adding, editing, moving, resizing and deleting
@@ -141,6 +142,7 @@ void QgsMapToolAnnotation::keyPressEvent( QKeyEvent* e )
mCanvas->scene()->removeItem( sItem );
delete sItem;
mCanvas->setCursor( neutralCursor );
QgsProject::instance()->setDirty( true ); // TODO QGIS3: Rework the whole annotation code to be MVC compliant, see PR #2506
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the note with a reference to this PR.

m-kuhn added a commit that referenced this pull request Nov 30, 2015
changes to annotations mark the project dirty (fixes #7586)
@m-kuhn m-kuhn merged commit a792a47 into qgis:master Nov 30, 2015
@m-kuhn
Copy link
Member

m-kuhn commented Nov 30, 2015

Thanks a lot @SebDieBln
As usual, good work :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants