Unverified Commit 49c47008 authored by Kyle Keating's avatar Kyle Keating Committed by GitHub

Prevent scrolling to annotations from links or buttons

If an event is triggered from a link or button or a descendent of a link or button, then prevent scrollToAnnotation from firing. It is assumed in these cases that a more specific action was intended and the page shall not scroll to the underlying annotation. Additional, dom-element.js has moved to /shared since its now used in both the annotator and sidebar 
parent 8cdf8bfb
...@@ -10,7 +10,7 @@ highlighter = require('./highlighter') ...@@ -10,7 +10,7 @@ highlighter = require('./highlighter')
rangeUtil = require('./range-util') rangeUtil = require('./range-util')
{ default: selections } = require('./selections') { default: selections } = require('./selections')
xpathRange = require('./anchoring/range') xpathRange = require('./anchoring/range')
{ closest } = require('./util/dom-element') { closest } = require('../shared/dom-element')
{ normalizeURI } = require('./util/url') { normalizeURI } = require('./util/url')
animationPromise = (fn) -> animationPromise = (fn) ->
......
import { closest } from './util/dom-element'; import { closest } from '../shared/dom-element';
const SVG_NAMESPACE = 'http://www.w3.org/2000/svg'; const SVG_NAMESPACE = 'http://www.w3.org/2000/svg';
......
import { closest } from '../dom-element'; import { closest } from '../dom-element';
describe('annotator/util/dom-element', () => { describe('shared/dom-element', () => {
let container; let container;
beforeEach(() => { beforeEach(() => {
......
...@@ -110,6 +110,23 @@ describe('ThreadCard', () => { ...@@ -110,6 +110,23 @@ describe('ThreadCard', () => {
assert.calledWith(fakeFrameSync.focusAnnotations, sinon.match([])); assert.calledWith(fakeFrameSync.focusAnnotations, sinon.match([]));
}); });
['button', 'a'].forEach(tag => {
it(`does not scroll to the annotation if the event's target or ancestor is a ${tag}`, () => {
const wrapper = createComponent();
const nodeTarget = document.createElement(tag);
const nodeChild = document.createElement('div');
nodeTarget.appendChild(nodeChild);
wrapper.find('.thread-card').props().onClick({
target: nodeTarget,
});
wrapper.find('.thread-card').props().onClick({
target: nodeChild,
});
assert.notCalled(fakeFrameSync.scrollToAnnotation);
});
});
}); });
it( it(
......
...@@ -4,6 +4,7 @@ import { useCallback } from 'preact/hooks'; ...@@ -4,6 +4,7 @@ import { useCallback } from 'preact/hooks';
import debounce from 'lodash.debounce'; import debounce from 'lodash.debounce';
import propTypes from 'prop-types'; import propTypes from 'prop-types';
import { closest } from '../../shared/dom-element';
import useStore from '../store/use-store'; import useStore from '../store/use-store';
import { withServices } from '../util/service-context'; import { withServices } from '../util/service-context';
...@@ -33,10 +34,24 @@ function ThreadCard({ frameSync, settings = {}, thread }) { ...@@ -33,10 +34,24 @@ function ThreadCard({ frameSync, settings = {}, thread }) {
[frameSync] [frameSync]
); );
/**
* Is the target's event an <a> or <button> element, or does it have
* either as an ancestor?
*/
const isFromButtonOrLink = target => {
return !!closest(target, 'button') || !!closest(target, 'a');
};
return ( return (
/* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */ /* eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions */
<div <div
onClick={() => scrollToAnnotation(threadTag)} onClick={e => {
// Prevent click events intended for another action from
// triggering a page scroll.
if (!isFromButtonOrLink(e.target)) {
scrollToAnnotation(threadTag);
}
}}
onMouseEnter={() => focusThreadAnnotation(threadTag)} onMouseEnter={() => focusThreadAnnotation(threadTag)}
onMouseLeave={() => focusThreadAnnotation(null)} onMouseLeave={() => focusThreadAnnotation(null)}
key={thread.id} key={thread.id}
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment