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

Feature/integration improvements #1043

Conversation

radpet
Copy link
Contributor

@radpet radpet commented Aug 11, 2016

This pull request fixes:

Memory leaks caused by not unsubscribing from events when object is not used any more
Handle refresh annotation action properly

This pull request adds:

Events that handle all annotation logic inside overlay in order to remove them from the annotation-tooltip object

Option whether to proceed to confirmation screen after save beeing clicked when you edit annotation and have no shapes.

@radpet radpet force-pushed the feature/integration-improvements branch from 274bb7f to c0313d6 Compare August 11, 2016 14:13
@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+0.3%) to 47.197% when pulling c0313d6 on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

@coveralls
Copy link

coveralls commented Aug 11, 2016

Coverage Status

Coverage increased (+0.3%) to 47.197% when pulling c0313d6 on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

};

if (_this.activeEditor.isDirty()) {
new $.DialogBuilder().confirm(i18n.t('cancelAnnotation'),function(result){
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like you are not passing the isDirty check in the event publisher so it's showing the dialog on an empty text editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks :)

@rsinghal
Copy link
Collaborator

@radpet I noticed (and I don't know if this happened in this PR or a previous one), but when you go into Edit mode for an annotation and click on one of the shapes and then click outside of it, it draws that shape. It should only draw a shape when the user explicitly selects one of the shapes by the top icons. A user should be able to click around the rest of the image when in pointer mode without drawing a shape. I believe this was working at one point, so I don't know when it broke.

@radpet
Copy link
Contributor Author

radpet commented Aug 12, 2016

@rsinghal

@radpet I noticed (and I don't know if this happened in this PR or a previous one), but when you go into Edit mode for an annotation and click on one of the shapes and then click outside of it, it draws that shape. It should only draw a shape when the user explicitly selects one of the shapes by the top icons. A user should be able to click around the rest of the image when in pointer mode without drawing a shape. I believe this was working at one point, so I don't know when it broke.

Nah this happens in annotation-work too... I'll try to fix it in another pull request.

@radpet radpet force-pushed the feature/integration-improvements branch from c0313d6 to a059793 Compare August 12, 2016 07:09
@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage increased (+0.3%) to 47.197% when pulling a059793 on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

@radpet radpet force-pushed the feature/integration-improvements branch from a059793 to 1b18335 Compare August 12, 2016 11:40
@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage increased (+0.3%) to 47.168% when pulling 1b18335 on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

@radpet radpet force-pushed the feature/integration-improvements branch from 1b18335 to 57ea361 Compare August 12, 2016 11:56
@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage increased (+0.3%) to 47.158% when pulling 57ea361 on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

@radpet radpet force-pushed the feature/integration-improvements branch from 57ea361 to 676306e Compare August 12, 2016 14:12
@coveralls
Copy link

coveralls commented Aug 12, 2016

Coverage Status

Coverage increased (+0.3%) to 47.138% when pulling 676306e on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

…vent name and handler

on subscribe in order unsubscripe only for the registered handler

Refactor draw-tool to unsubscribe from events when osd is destroyed
Refactor overlay to not remove all osd handlers when destroyed
Refactor annotationTooltip to not include any save/create/edit annotation logic
AvailableExternalComments setting now shows tooltip
Go to pointer mode when refresh is clicked
@radpet radpet force-pushed the feature/integration-improvements branch from 676306e to bb4edbc Compare August 12, 2016 14:59
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 47.168% when pulling bb4edbc on SirmaITT:feature/integration-improvements into 5d67ac2 on IIIF:annotation-work.

@rsinghal
Copy link
Collaborator

We need that bug fixed as soon as possible as we will be doing a code freeze early next week.

@radpet
Copy link
Contributor Author

radpet commented Aug 12, 2016

Right I will start fixing it right now, meanwhile please check this pull request.

@rsinghal rsinghal merged commit 5cae633 into ProjectMirador:annotation-work Aug 12, 2016
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