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.

Sign up for GitHub

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

Jump to bottom

Fix: Pass event to resetHighlightSelection to reset dialog #129

Merged
merged 3 commits into from
Mar 6, 2018
Merged

Fix: Pass event to resetHighlightSelection to reset dialog #129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
627eaa0
Fix: Pass event to resetHighlightSelection to reset dialog
pramodsum Mar 6, 2018
209051b
Fix: mobile CSS classes from bp to ba
pramodsum Mar 6, 2018
8d2eac5
Fix: tests
pramodsum Mar 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/MobileAnnotator.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@
}

.ba-mobile-annotation-dialog .ba-annotation-highlight-btns,
.ba-mobile-annotation-dialog .bp-create-comment,
.ba-mobile-annotation-dialog .ba-create-comment,
.ba-mobile-annotation-dialog .annotation-container section[data-section="create"],
.ba-mobile-create-annotation-dialog {
background: white;
Expand Down Expand Up @@ -247,7 +247,7 @@
}
}

.bp-container .bp-annotate-draw-undo-redo-container {
.bp-container .ba-draw-undo-redo-container {
margin: 0;

@media #{$tablet} {
Expand Down
6 changes: 3 additions & 3 deletions src/doc/DocAnnotator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ class DocAnnotator extends Annotator {
*/
resetHighlightSelection(event) {
const isCreateDialogVisible = this.createHighlightDialog && this.createHighlightDialog.isVisible;
if (!isCreateDialogVisible || util.isInDialog(event)) {
if (!isCreateDialogVisible || !event || util.isInDialog(event)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Question about this: Does this mean you can no longer remove highlight selection in a function that isn't triggered by a browser event?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. There are still one or 2 occasions where the highlight selection is removed without destroying the dialog. This is more for automatically destroying the dialog AND clearing the selection

return;
}

Expand Down Expand Up @@ -852,7 +852,7 @@ class DocAnnotator extends Annotator {
this.highlighter.removeAllHighlights();
}

this.resetHighlightSelection();
this.resetHighlightSelection(event);
this.isCreatingHighlight = false;

// Prevent the creation of highlights if the user is currently creating a point annotation
Expand Down Expand Up @@ -1026,7 +1026,7 @@ class DocAnnotator extends Annotator {

switch (data.event) {
case CONTROLLER_EVENT.toggleMode:
this.resetHighlightSelection();
this.resetHighlightSelection(data.event);
break;
case CONTROLLER_EVENT.bindDOMListeners:
if (isCreateDialogVisible) {
Expand Down
4 changes: 2 additions & 2 deletions src/doc/__tests__/DocAnnotator-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,7 +836,7 @@ describe('doc/DocAnnotator', () => {
});

it('should do nothing if the createHighlightDialog is hidden', () => {
annotator.resetHighlightSelection();
annotator.resetHighlightSelection({});
expect(annotator.createHighlightDialog.hide).to.not.be.called;
});

Expand All @@ -848,7 +848,7 @@ describe('doc/DocAnnotator', () => {

it('hide the visible createHighlightDialog and clear the text selection', () => {
annotator.createHighlightDialog.isVisible = true;
annotator.resetHighlightSelection();
annotator.resetHighlightSelection({});
expect(annotator.createHighlightDialog.hide).to.be.called;
});
});
Expand Down