Skip to content

Commit

Permalink
Fix: Creation of highlights on Edge/IE11/Firefox browsers (#322)
Browse files Browse the repository at this point in the history
- Fixes text selection issues on IE/Edge
- Prevent focus change from removing highlight selection
- Bail if click is outside selection/highlight
- Double click to highlight on Edge
- Remove thread.scrollIntoView() call on render
  • Loading branch information
pramodsum authored Jan 21, 2019
1 parent daf18a7 commit 3d9ee7f
Show file tree
Hide file tree
Showing 13 changed files with 175 additions and 327 deletions.
14 changes: 14 additions & 0 deletions src/__tests__/AnnotationThread-test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable no-unused-expressions */
import * as ReactDOM from 'react-dom';
import AnnotationThread from '../AnnotationThread';
import * as util from '../util';
import {
Expand Down Expand Up @@ -117,6 +118,19 @@ describe('AnnotationThread', () => {
});
});

describe('renderAnnotationPopover()', () => {
it('should render and display the popover for this annotation', () => {
thread.getPopoverParent = jest.fn().mockReturnValue(rootElement);
util.getPopoverLayer = jest.fn().mockReturnValue(rootElement);
util.shouldDisplayMobileUI = jest.fn().mockReturnValue(false);
ReactDOM.render = jest.fn();
thread.position = jest.fn();

thread.renderAnnotationPopover();
expect(thread.popoverComponent).not.toBeUndefined();
});
});

describe('hide()', () => {
it('should hide the thread element', () => {
thread.unmountPopover = jest.fn();
Expand Down
7 changes: 0 additions & 7 deletions src/_common.scss
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@
src: local('Lato Bold'), local('Lato-Bold'), url('https://cdn01.boxcdn.net/fonts/1.0.2/lato/Lato-Bold.woff2') format('woff2'), url('https://cdn01.boxcdn.net/fonts/1.0.2/lato/Lato-Bold.woff') format('woff');
}

@import '~box-react-ui/lib/styles/common/overlay';

.ba {
@include reset;

Expand Down Expand Up @@ -96,11 +94,6 @@
@import '~box-react-ui/lib/styles/common/buttons';
}

.flyout-overlay {
@import '~box-react-ui/lib/styles/common/links';
@import '~box-react-ui/lib/styles/common/buttons';
}

/**************************************
* Accessibility
**************************************/
Expand Down
6 changes: 3 additions & 3 deletions src/components/AnnotationPopover/AnnotationPopover.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import PlainButton from 'box-react-ui/lib/components/plain-button';
import IconClose from 'box-react-ui/lib/icons/general/IconClose';

import Internationalize from '../Internationalize';
import Overlay from './Overlay';
import CommentList from '../CommentList';
import { TYPES, CLASS_ANNOTATION_POPOVER, CLASS_ANNOTATION_CARET } from '../../constants';

Expand All @@ -19,6 +18,7 @@ const CLASS_ANIMATE_POPOVER = 'ba-animate-popover';
const CLASS_CREATE_POPOVER = 'ba-create-popover';
const CLASS_MOBILE_HEADER = 'ba-mobile-header';
const CLASS_MOBILE_CLOSE_BTN = 'ba-mobile-close-btn';
const CLASS_POPOVER_OVERLAY = 'ba-popover-overlay';

type Props = {
isMobile: boolean,
Expand Down Expand Up @@ -99,7 +99,7 @@ class AnnotationPopover extends React.PureComponent<Props> {
) : (
<span className={CLASS_ANNOTATION_CARET} />
)}
<Overlay shouldDefaultFocus={!isMobile}>
<div className={CLASS_POPOVER_OVERLAY}>
{hasComments ? (
<CommentList comments={comments} onDelete={onDelete} />
) : (
Expand All @@ -121,7 +121,7 @@ class AnnotationPopover extends React.PureComponent<Props> {
onCommentClick={onCommentClick}
/>
)}
</Overlay>
</div>
</div>
</Internationalize>
);
Expand Down
5 changes: 3 additions & 2 deletions src/components/AnnotationPopover/AnnotationPopover.scss
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@
}
}

.overlay {
.ba-popover-overlay {
background: white;
border: 1px solid $seesee;
border-radius: 4px;
display: block;
Expand All @@ -50,7 +51,7 @@
white-space: normal;
}

.ba-inline-popover .overlay {
.ba-inline-popover .ba-popover-overlay {
align-items: center;
display: inline-flex;
}
Expand Down
25 changes: 0 additions & 25 deletions src/components/AnnotationPopover/Overlay.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -73,24 +73,14 @@ describe('components/AnnotationPopover', () => {
expect(wrapper.find('.ba-inline-popover').length).toEqual(1);
});

test('should correctly render a BRUI Overlay if not on mobile', () => {
const wrapper = render({
canAnnotate: true,
comments,
isMobile: false
});
expect(wrapper).toMatchSnapshot();
expect(wrapper.find('Overlay').prop('shouldDefaultFocus')).toBeTruthy();
});

test('should correctly render a div without a Focus Trap if on mobile', () => {
test('should correctly render a div without a Focus Trap', () => {
const wrapper = render({
canAnnotate: true,
comments,
isMobile: true
});
expect(wrapper).toMatchSnapshot();
expect(wrapper.find('Overlay').prop('shouldDefaultFocus')).toBeFalsy();
expect(wrapper.find('Overlay').exists()).toBeFalsy();
});

test('should correctly render a popover with comments and reply textarea', () => {
Expand Down
18 changes: 0 additions & 18 deletions src/components/AnnotationPopover/__tests__/Overlay-test.js

This file was deleted.

Loading

0 comments on commit 3d9ee7f

Please sign in to comment.