Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
leaflet: fixed DOM leaking on annotation remove in mobile
An extra annotation element was added but never removed
This was not a functional problem
But not a good practice and also made cypress hard to maintain

Change-Id: I53fc7d55154a601c52f89e35b9236f5ae66b46fd
Signed-off-by: Pranam Lashkari <lpranam@collabora.com>
  • Loading branch information
lpranam authored and tzolnai committed Nov 2, 2020
1 parent 0d031e1 commit 580eecf
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 6 deletions.
Expand Up @@ -96,10 +96,10 @@ describe('Annotation tests.', function() {
cy.get('.loleaflet-annotation')
.should('exist');

cy.get('.loleaflet-annotation:nth-of-type(2) .loleaflet-annotation-content')
cy.get('.loleaflet-annotation:nth-of-type(1) .loleaflet-annotation-content')
.should('have.text', 'some text');

cy.get('.loleaflet-annotation:nth-of-type(3) .loleaflet-annotation-content')
cy.get('.loleaflet-annotation:nth-of-type(2) .loleaflet-annotation-content')
.should('have.text', 'reply');
});

Expand All @@ -112,7 +112,8 @@ describe('Annotation tests.', function() {
mobileHelper.selectAnnotationMenuItem('Remove');

cy.get('.loleaflet-annotation-content')
.should('have.text', '');
.should('not.exist');

});

it('Try to insert empty comment.', function() {
Expand Down
Expand Up @@ -726,21 +726,21 @@ describe('Trigger hamburger menu options.', function() {
cy.get('.vex-dialog-button-primary')
.click();

cy.get('.loleaflet-annotation:nth-of-type(2)')
cy.get('.loleaflet-annotation')
.should('have.attr', 'style')
.should('not.contain', 'visibility: hidden');

// Resolve comment
mobileHelper.selectAnnotationMenuItem('Resolve');

cy.get('.loleaflet-annotation:nth-of-type(2)')
cy.get('.loleaflet-annotation')
.should('have.attr', 'style')
.should('contain', 'visibility: hidden');

// Show resolved comments
mobileHelper.selectHamburgerMenuItem(['View', 'Resolved Comments']);

cy.get('.loleaflet-annotation:nth-of-type(2)')
cy.get('.loleaflet-annotation')
.should('have.attr', 'style')
.should('not.contain', 'visibility: hidden');

Expand Down
2 changes: 2 additions & 0 deletions loleaflet/src/layer/tile/TileLayer.js
Expand Up @@ -438,6 +438,8 @@ L.TileLayer = L.GridLayer.extend({

// FIXME: Unify annotation code in all modules...
addCommentFn.call(that, {annotation: annotation}, comment);
if (!isMod)
that._map.removeLayer(annotation);
}
}
});
Expand Down

0 comments on commit 580eecf

Please sign in to comment.