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

Animate annotation dialog #161

Merged
merged 11 commits into from
Jun 6, 2017
Merged

Animate annotation dialog #161

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5edc0d4
Chore: Animate annotation dialog on mobile
Jun 1, 2017
26e7f6a
Chore: Media query for large devices
Jun 1, 2017
36525b9
Chore: Media query for large devices
Jun 1, 2017
7c0614e
Chore: Amended when I should not have
Jun 1, 2017
38710f5
Merge branch 'Animate-annotation-dialog' of github.com:JustinHoldstoc…
Jun 1, 2017
620e4f9
Merge branch 'master' into Animate-annotation-dialog
Jun 2, 2017
a7720d0
Merge branch 'master' of github.com:JustinHoldstock/box-content-previ…
Jun 5, 2017
b17a5d8
Merge branch 'Animate-annotation-dialog' of github.com:JustinHoldstoc…
Jun 5, 2017
2b83945
Chore: Animate highlight dialog
Jun 6, 2017
a69c1c7
Chore: Linting for animation before border
Jun 6, 2017
417a8ff
Merge branch 'master' of github.com:JustinHoldstock/box-content-previ…
Jun 6, 2017
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
6 changes: 6 additions & 0 deletions src/lib/annotations/AnnotationDialog.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import { CLASS_ACTIVE, CLASS_HIDDEN } from '../constants';
import { decodeKeydown } from '../util';
import { ICON_CLOSE, ICON_DELETE } from '../icons/icons';

const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';

@autobind class AnnotationDialog extends EventEmitter {
//--------------------------------------------------------------------------
// Typedef
Expand Down Expand Up @@ -80,6 +82,8 @@ import { ICON_CLOSE, ICON_DELETE } from '../icons/icons';
const dialogCloseButtonEl = this.element.querySelector('.bp-annotation-dialog-close');
dialogCloseButtonEl.addEventListener('click', this.hideMobileDialog);

this.element.classList.add(CLASS_ANIMATE_DIALOG);

this.bindDOMListeners();
}

Expand Down Expand Up @@ -133,6 +137,8 @@ import { ICON_CLOSE, ICON_DELETE } from '../icons/icons';
return;
}

this.element.classList.remove(CLASS_ANIMATE_DIALOG);

// Clear annotations from dialog
this.element.innerHTML = `
<div class="bp-annotation-mobile-header">
Expand Down
37 changes: 37 additions & 0 deletions src/lib/annotations/MobileAnnotator.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,46 @@
$tablet: "(min-width: 768px)";
Copy link
Contributor

Choose a reason for hiding this comment

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

is 768 a standard table size? Or is just applicable to our situation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the roughly the break point for an iPhone 6s (but still not as wide as a tablet in portrait), when rotated to landscape mode. I figured that anything larger than that would be considered "not fullscreen-able"


.bp-mobile-annotation-dialog {
background: white;
border-top: 0;
height: 100%;
top: 0;
position: absolute;
width: 100%; // Hard-coded width to solve layout issues

&.bp-animate-show-dialog:not(.bp-plain-highlight) {
animation: show-dialog-small;
animation-duration: 0.2s;
animation-fill-mode: forwards;

@media #{$tablet} {
width: 450px;
animation: show-dialog-tablet;
animation-duration: 0.2s;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note the duplicated animation duration and fill-mode.
These are nuked when applying the media query

animation-fill-mode: forwards;
}

}
}

@keyframes show-dialog-small {
0% {
top: 100%;
}

100% {
top: 0%;
}
}

@keyframes show-dialog-tablet {
0% {
right: -50%;
}

100% {
right: 0%;
}
}

.bp-mobile-annotation-dialog.bp-annotation-dialog {
Expand Down
17 changes: 17 additions & 0 deletions src/lib/annotations/__tests__/AnnotationDialog-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import * as annotatorUtil from '../annotatorUtil';
import * as constants from '../annotationConstants';
import { CLASS_ACTIVE, CLASS_HIDDEN } from '../../constants';

const CLASS_ANIMATE_DIALOG = 'bp-animate-show-dialog';

let dialog;
const sandbox = sinon.sandbox.create();
let stubs = {};
Expand Down Expand Up @@ -147,6 +149,14 @@ describe('lib/annotations/AnnotationDialog', () => {
expect(stubs.show).to.be.calledWith(dialog.element);
expect(stubs.bind).to.be.called;
expect(dialog.position).to.not.be.called;
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.true;
});

it('should add the animation class to the the mobile dialog if using a mobile browser', () => {
dialog.isMobile = true;

dialog.show();
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.true;
});

it('should hide the mobile header if a plain highlight', () => {
Expand Down Expand Up @@ -174,6 +184,13 @@ describe('lib/annotations/AnnotationDialog', () => {
stubs.hide = sandbox.stub(annotatorUtil, 'hideElement');
dialog.hideMobileDialog();
expect(stubs.hide).to.be.called;
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false;
});

it('should remove the animation class', () => {
dialog.element = document.querySelector('.bp-mobile-annotation-dialog');
dialog.hideMobileDialog();
expect(dialog.element.classList.contains(CLASS_ANIMATE_DIALOG)).to.be.false;
});
});

Expand Down