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

feat(highlight): Change cursor in Highlight Text mode #544

Merged
merged 3 commits into from
Jul 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions src/document/DocumentAnnotator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import BaseAnnotator from '../common/BaseAnnotator';
import BaseManager from '../common/BaseManager';
import { centerRegion, isRegion, RegionManager } from '../region';
import { getAnnotation } from '../store/annotations';
import { HighlightManager } from '../highlight';
import { scrollToLocation } from '../utils/scroll';
import './DocumentAnnotator.scss';

Expand Down Expand Up @@ -30,6 +31,7 @@ export default class DocumentAnnotator extends BaseAnnotator {
// Lazily instantiate managers as pages are added or re-rendered
if (managers.size === 0) {
managers.add(new RegionManager({ location: pageNumber, referenceEl: pageReferenceEl }));
managers.add(new HighlightManager({ location: pageNumber, referenceEl: pageReferenceEl }));
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
}

return managers;
Expand Down
3 changes: 2 additions & 1 deletion src/document/__tests__/DocumentAnnotator-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { annotations as regions } from '../../region/__mocks__/data';
import { fetchAnnotationsAction } from '../../store';
import { scrollToLocation } from '../../utils/scroll';

jest.mock('../../highlight/HighlightManager');
jest.mock('../../region/RegionManager');
jest.mock('../../utils/scroll');

Expand Down Expand Up @@ -69,7 +70,7 @@ describe('DocumentAnnotator', () => {
test('should create new managers given a new page element', () => {
const managers = annotator.getPageManagers(getPage());

expect(managers.size).toBe(1);
expect(managers.size).toBe(2);
expect(managers.values().next().value).toBeInstanceOf(RegionManager);
});

Expand Down
8 changes: 8 additions & 0 deletions src/highlight/HighlightAnnotations.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
.ba-HighlightAnnotations-creator {
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
pointer-events: auto;
jstoffan marked this conversation as resolved.
Show resolved Hide resolved
}
20 changes: 20 additions & 0 deletions src/highlight/HighlightAnnotations.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import * as React from 'react';
import HighlightCreator from './HighlightCreator';

import './HighlightAnnotations.scss';

type Props = {
isCreating: boolean;
};

export default class HighlightAnnotations extends React.PureComponent<Props> {
static defaultProps = {
isCreating: false,
};

render(): JSX.Element {
const { isCreating } = this.props;

return <>{isCreating && <HighlightCreator className="ba-HighlightAnnotations-creator" />}</>;
}
}
14 changes: 14 additions & 0 deletions src/highlight/HighlightContainer.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { connect } from 'react-redux';
import { AppState, getAnnotationMode } from '../store';
import HighlightAnnotations from './HighlightAnnotations';
import withProviders from '../common/withProviders';

export type Props = {
isCreating: boolean;
};

export const mapStateToProps = (state: AppState): Props => ({
isCreating: getAnnotationMode(state) === 'highlight',
});

export default connect(mapStateToProps)(withProviders(HighlightAnnotations));
8 changes: 8 additions & 0 deletions src/highlight/HighlightCreator.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
$text_cursor_32: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAACAAAAAgCAYAAABzenr0AAAABGdBTUEAALGPC/xhBQAAADhlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAAqACAAQAAAABAAAAIKADAAQAAAABAAAAIAAAAACPTkDJAAABrklEQVRYCWNgGAWjITAaAqMhQCAEfv36NeX///9/gBgO/v379/f716/TCWglSpqRkKq/f//+tjAxYfn8+TNcKS8vL8OJM2f+MDMzs8IFacUA+RTkY5D3NVRUYKHwBxQytLITq7nIDgBa/g3mEmz0169fP3z58qUfKMeG1TAkQRYkNlHMT58+/Q3w8eF8/vw5TvXiEhL8nd3daTq6uiA1hTgVAiWY8Elik3v86NFnZMvZ2NgY/AMCGOISEhgEBAXBWl6+eMFQUVrKxcjImIjNDGQxkh2ArBnETkpJYdDS1mYwMjb+P3vu3N8w+RdAR3BxcfHD+Lhoih0wY9o0huVLlzIcO3KEUVtHh+QoJVkDuk8mTpnyhZOT89/ZM2f4gEFOMFuj66c4BGxsbVnu3r37WkxMDB786Jbg41McAknx8RzmFhbKRw8fZrh69SrcLgkJCYZv3759hAvgYJDsAFk5OV5JSUkGWE64eOECAwiDAMwBwGzI0NHdDSor5uOwl3RhUIEDKwl//vz5HcTHBUgpiAi6BLkygjlg5FZGSEFO/8qIYFyNKhgNgdEQGKohAAD+bzKe7UZRtgAAAABJRU5ErkJggg==';
$text_cursor_32_2x: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAEAAAABACAYAAACqaXHeAAAABGdBTUEAALGPC/xhBQAAADhlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAAqACAAQAAAABAAAAQKADAAQAAAABAAAAQAAAAABlmWCKAAADvElEQVR4Ae2ZzUsbQRTA37a1BSkVhNjEg400h2ouQiWpaFIpftGmxP4FQqs9RLykiIj0JD3YIgVvFnvrUYppUq0kSk9Wi0URcrNgtGCKFyW9JKHZvrfNVjTJmt3R3bSdBy+zO/ve7rzfvJnMzgJw4QQ4AU6AE+AEOAFOgBPgBDgBToAT4AQ4AU5AJQFRFGtQw6hJ1JOEbMi2RuVjStc8G9BJgR+/Hi6ViATWhmBkSbzHxVuNjXBwcKB4u4qKClheXSWblCAIlxSNdbp4Tqfn/LuPKTQEbthsImkBKZkhcBoZ8Ai7N4KaKqKbyYZsyackhBkAjuVt1HYa06hKc8qXbMRtWMYKZIZSdQIvLqP2oSo9RxVYZgAqnnYTbS+qsD9uehkrnKivUCMIwXLcQMv5BS1Oan3SqZT4xO8XPi4uQjqdVusu2ZtMJnA4nTA4NCReNZvvYOUbhNCGSSdqumHWSZcMCMzMCOH5ec3BU1v39vbgfSgEXo9H+B6PU9AEoZeusYguAOLxOEsbj/jSWuPF2Jg8BzBPproMgSMR5Dmx1taCr78f6urrwVJdDVtbWzA1OQkf5ubyWAN8XlmR6+3ygdZSlwxQapzL7YZAMAj3vV4wVVWBmMmA3W6HlxMT0NHVldeVhkNWaGJkEsMBrK+twbtAAAZ8PqDldEtTE7ydnpaC6u3rYwquGGfDASQSCXg6MgKR8O/FYTKZhLnZWant1222YmJgsimJOYAisGGw7tZWuGa1ghWVpLy8XCrP8sdwAJWVldJ4p//4TCYjxmIxIYVZoJcYPgSej49LC5yN9fWNZqdTuNvRAc9GR/WKHwwH0NDQIAW7EInE9vf3pePmlhbdABg+BKLRqJQBvoGB23X490cTX7XF8hMJUOfIC54zA2J4BowMD8OnpSUK8IrL5YKvm5vwwOs9HwoGhfjubt7Aab2QlR/ygdbS8Az4trMDD3t6cto/6Pfn1MkVDodDPozKB1pLXTLAbDZrbV+OH+0r0hth9sLrHAOVFboA8HZ3i+2dnVBWVqayeYfm9Dp8z+OBQChEr8M0NyyiTh1alMgRvqNLcsKeoGymtVxAx1PZENElA7J9Q1tixewbFupKmvDoNfAxKm2E5J8hC3kXqGeeBLEn6CsPjUU3qtKWF22J/dkUxQC28dxwOY0MoODbUJWClwMlG7JlnrzkG7KWzAsNzAD+ZYi1F/5qf8wA+tqrVkrmyxAzfIz8//48zkyQ34AT4AQ4AU6AE+AEOAFOgBPgBDgBToAT4AT0JvAL80N/mFc/WlQAAAAASUVORK5CYII=';
$text_cursor_32_3x: 'data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAGAAAABgCAYAAADimHc4AAAABGdBTUEAALGPC/xhBQAAADhlWElmTU0AKgAAAAgAAYdpAAQAAAABAAAAGgAAAAAAAqACAAQAAAABAAAAYKADAAQAAAABAAAAYAAAAACK+310AAAF6UlEQVR4Ae2cXUwVRxSAD0QJP8Wr/Ib0pfy1/JjKBdpLUiuGy080tbWtYBoTI0YhfTM1KYmJbXxsE19sYpomNtH0RUkDNE2tCCq3osBDoU88SGMT24KExPIjoRBCz7ll1lH2/uy9O3t3L+ckJzt7Z+bMzHdmZ2Z3di8ACxNgAkyACTABJsAEmAATYAJMgAkwASbABJgAE2ACTIAJMAEmwASYABNgAkyACTABJsAEmAATYAJMIAICa2trbtRe1FnUSITyUX53BMVv7iwEDfVfVDOE7LATjHQpBEY910zpNVK+3dMmqK4gkp/FMrZROe/s2we/T0wYLrKwqAh+vH5d5JtLSEhwiROnH61wwJqAVFpcLIKGj+MPHmh50AHK660VpjiQqNg+mZ8TZVBPjkReyKfZi8SW3fIo70k4BNGY3RCs4fKVIff0AHlu4gXQGCDOcT9bcQV0IJVlk8iQHbIXN6LcAdhbR5FWDepN1EiHD8pH+WvW7WEwPkT5EBQIE61LRVyQIegnTONBzRRpTT4uoL2/UH2oV9G5/SbbD2lO+RUQsgbBE+zHaFXwqeSXUF9DPYnah33iDmrkSzU0YlTs7gCj7Yk2fS0aGEYnvB2toXDzbwk3YSzS9XR3w9cXL8IfDx8qKT41NRWK8N7kw+ZmaG5pARyCqJwdqD3oBA+eP7v5UFIDANvOAb6BAWg/cUJRszea9dTUwJfnz0NOTo6IHEAH7BUnqo62HYJ+wN5vpQwPDcGnp08D9nxRbC2GveJE1dG2DpienlbV5oB2yQmd167J8YflExVhW88BRhucmJgIr+/aBSWlpZCeng5/PnoE4+PjhuaQ7zs7oeWwxn2P0ToYTR8XDkhKSoKjx45B6/HjkJG5cdXae+MGfH72LPzz5ElIPhPSQz9M/HLIDFEmcLwDMhH4N5cuQVl5eUAUjU1NkF9QAO8fOACrq6sB01HE4uKiHE/3CUrF8Q6Yn5+HHRkZfkjLy8tAk/fIyAg8XVgAb309fHDokD+uGJebHx05At9duaIUqFHjjncAQW/DoWdvXR38jJs2NO4LudXfD1nZ2bCnlu6vAOobGtgBAo6ZxwncZSPVk/uDg5oDCgoL9ZLE9DfbLkPNojI79+wBbDZeDXYTxw9BMlB3ZSXUeb3+CbkAJ13X9u2QkpIiJ7FdOC4cUFFRAZ+dOwelZWW2AxyqQo53QFV1NXx7+TLQvQDJ5OTk056urrTfxsZg+vFjeNPjgY4zZ0JxiFm84x1wsr1dhj/c5PV6VlZWNKCvlpRoYTsGHD8JV1ZVaVzv37v3qwyfIl7Jz9fi7Rhw/BVAwww99yF5a/fuN/Ly8mBqagoKccl5tLXV/5zfjuBFnRzvALrZEu8N5ebmVt/y+WBpaQmSk5P9bZyZmZnLysqiRwq2vNptWSnRO8I5fnXhAty5ffu5pASfnut3d3XB/sbGbTg0hd3O1LQ02da8fKIi7PgrYAUfRXzc1gYVbjeU79wJLpfLv/oZvHuXVkR+Zp+cOgVVOFds2bo1JMOi59/e+ztkhigTON4Bov1jo6NAqif0GLq/r08vasNvtD8siU8KKwmGfWkqKT2IUWlvNkgqc6NoX5g25yW5KoWVBG3rgHcPHlTS4EBGxaY8bsSLJLQpr/xFLa00UapVR5wktd3vQG/Gxei1FEJAW2eb+7UUqzqCTjkE/z3s/b/oxJn+k22HINNbGp7BAUxGPd8S+FQlu6+C4v7lXEscgMM9fdn4BfUuVP/3YngMR+jlXBLaVRlG7cDeqb/WpFQOFOUOWIc/hGz+f14cGSRyWgNqLdqLq28ErJgDqOdHA192Gdkhe3Ejypeh2GP5M9Ug3cUKB+iu94PUSTdK/ngP5wHl9dathIIfrRiCtNcSxGNjo+14IZ9mz6idTZkehyD+q4JYeh4dwH/WEUsHUNnrTuC/q4m1I7h8JsAEmAATYAJMgAkwASbABJgAE2ACTIAJMAEmwASYABNgAkyACTABJsAEmAATYAJMgAkwASbABCwh8B+n1k4/vWVUjgAAAABJRU5ErkJggg==';

.ba-HighlightCreator {
cursor: url($text_cursor_32) 16 16, text; /* Legacy */
cursor: image-set(url($text_cursor_32) 1x, url($text_cursor_32_2x) 2x, url($text_cursor_32_3x) 3x) 16 16, text; /* Webkit */ /* stylelint-disable-line */
}
11 changes: 11 additions & 0 deletions src/highlight/HighlightCreator.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import * as React from 'react';
import classNames from 'classnames';
import './HighlightCreator.scss';

type Props = {
className?: string;
};

export default function HighlightCreator({ className }: Props): JSX.Element {
return <div className={classNames(className, 'ba-HighlightCreator')} data-testid="ba-HighlightCreator" />;
}
49 changes: 49 additions & 0 deletions src/highlight/HighlightManager.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import BaseManager, { Options, Props } from '../common/BaseManager';
import HighlightContainer from './HighlightContainer';

export default class HighlightManager implements BaseManager {
location: number;

reactEl: HTMLElement;

constructor({ location = 1, referenceEl }: Options) {
this.location = location;
this.reactEl = this.insert(referenceEl);
}

destroy(): void {
ReactDOM.unmountComponentAtNode(this.reactEl);

this.reactEl.remove();
}

exists(parentEl: HTMLElement): boolean {
return parentEl.contains(this.reactEl);
}

insert(referenceEl: HTMLElement): HTMLElement {
// Find the nearest applicable reference and document elements
const documentEl = referenceEl.ownerDocument || document;
const parentEl = referenceEl.parentNode || documentEl;

// Construct a layer element where we can inject a root React component
const rootLayerEl = documentEl.createElement('div');
rootLayerEl.classList.add('ba-Layer');
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we have any concerns about rendering this layer even though the feature is not yet available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the toolbar highlight button is not clicked, we actually only render a <div /> with absolute positioning and pointer-event: none. So I don't have any concerns.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Gotcha. I think @ConradJChan had some concerns about rendering these components unconditionally, but I don't recall why. It should be okay for now, but we'll need to keep this in mind if we add anything else outside of the "creator" components.

rootLayerEl.classList.add('ba-Layer--highlight');
rootLayerEl.dataset.testid = 'ba-Layer--highlight';
rootLayerEl.setAttribute('data-resin-feature', 'annotations');

// Insert the new layer element immediately after the reference element
return parentEl.insertBefore(rootLayerEl, referenceEl.nextSibling);
}

render(props: Props): void {
ReactDOM.render(<HighlightContainer {...props} />, this.reactEl);
}

style(styles: Partial<CSSStyleDeclaration>): CSSStyleDeclaration {
return Object.assign(this.reactEl.style, styles);
}
}
29 changes: 29 additions & 0 deletions src/highlight/__tests__/HighlightAnnotations-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import HighlightAnnotations from '../HighlightAnnotations';
import HighlightCreator from '../HighlightCreator';

jest.mock('../HighlightCreator');

describe('components/highlight/HighlightAnnotations', () => {
const defaults = {
isCreating: false,
};

const getWrapper = (props = {}): ShallowWrapper => shallow(<HighlightAnnotations {...defaults} {...props} />);

describe('render()', () => {
test('should render a RegionCreator if in creation mode', () => {
const wrapper = getWrapper({ isCreating: true });
const creator = wrapper.find(HighlightCreator);

expect(creator.hasClass('ba-HighlightAnnotations-creator')).toBe(true);
});

test('should not render creation components if not in creation mode', () => {
const wrapper = getWrapper({ isCreating: false });

expect(wrapper.exists(HighlightCreator)).toBe(false);
});
});
});
29 changes: 29 additions & 0 deletions src/highlight/__tests__/HighlightContainer-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import * as React from 'react';
import { IntlShape } from 'react-intl';
import { mount, ReactWrapper } from 'enzyme';
import HighlightAnnotations from '../HighlightAnnotations';
import HighlightContainer, { Props } from '../HighlightContainer';
import { createStore } from '../../store';

jest.mock('../../common/withProviders');
jest.mock('../HighlightAnnotations');

describe('HighlightContainer', () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we follow the path format here? I'd also be game for nuking the path entirely and just using the component name. Maintaining the path here seems tedious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will nuke the path entirely in the followup pr.

const defaults = {
intl: {} as IntlShape,
location: 1,
store: createStore(),
};
const getWrapper = (props = {}): ReactWrapper<Props> => mount(<HighlightContainer {...defaults} {...props} />);

describe('render', () => {
test('should connect the underlying component and wrap it with a root provider', () => {
const wrapper = getWrapper();

expect(wrapper.exists('RootProvider')).toBe(true);
expect(wrapper.find(HighlightAnnotations).props()).toMatchObject({
isCreating: false,
});
});
});
});
15 changes: 15 additions & 0 deletions src/highlight/__tests__/HighlightCreator-test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import React from 'react';
import { shallow, ShallowWrapper } from 'enzyme';
import HighlightCreator from '../HighlightCreator';

describe('HighlightCreator', () => {
const getWrapper = (props = {}): ShallowWrapper => shallow(<HighlightCreator {...props} />);

describe('render', () => {
test('should add class', () => {
const wrapper = getWrapper();

expect(wrapper.hasClass('ba-HighlightCreator')).toBe(true);
});
});
});
75 changes: 75 additions & 0 deletions src/highlight/__tests__/HighlightManager-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
import ReactDOM from 'react-dom';
import { createIntl } from 'react-intl';
import HighlightManager from '../HighlightManager';
import { createStore } from '../../store';
import { Options } from '../../common/BaseManager';

jest.mock('react-dom', () => ({
render: jest.fn(),
unmountComponentAtNode: jest.fn(),
}));

describe('HighlightManager', () => {
const intl = createIntl({ locale: 'en' });
const rootEl = document.createElement('div');
const getOptions = (options: Partial<Options> = {}): Options => ({
referenceEl: rootEl.querySelector('.reference') as HTMLElement,
...options,
});
const getLayer = (): HTMLElement => rootEl.querySelector('[data-testid="ba-Layer--highlight"]') as HTMLElement;
const getWrapper = (options?: Partial<Options>): HighlightManager => new HighlightManager(getOptions(options));

beforeEach(() => {
rootEl.classList.add('root');
rootEl.innerHTML = '<div class="reference" />'; // referenceEl
});

describe('constructor', () => {
test('should set all necessary properties', () => {
const wrapper = getWrapper();

expect(wrapper.location).toEqual(1);
expect(wrapper.reactEl).toEqual(getLayer());
});
});

describe('destroy()', () => {
test('should unmount the React node and remove the root element', () => {
const wrapper = getWrapper();

wrapper.destroy();

expect(ReactDOM.unmountComponentAtNode).toHaveBeenCalledWith(wrapper.reactEl);
});
});

describe('exists()', () => {
test('should return a boolean based on its presence in the page element', () => {
const wrapper = getWrapper();

expect(wrapper.exists(rootEl)).toBe(true);
expect(wrapper.exists(document.createElement('div'))).toBe(false);
});
});

describe('render()', () => {
test('should format the props and pass them to the underlying components', () => {
const wrapper = getWrapper();

wrapper.render({ intl, store: createStore() });

expect(ReactDOM.render).toHaveBeenCalled();
});
});

describe('style', () => {
test('should assign the style object to the root element', () => {
const wrapper = getWrapper();

wrapper.style({ left: '5px', top: '10px' });

expect(getLayer().style.left).toEqual('5px');
expect(getLayer().style.top).toEqual('10px');
});
});
});
1 change: 1 addition & 0 deletions src/highlight/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { default as HighlightManager } from './HighlightManager';
1 change: 1 addition & 0 deletions src/store/common/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export enum Mode {
HIGHLIGHT = 'highlight',
NONE = 'none',
REGION = 'region',
}
Expand Down