Commit cc9a4723 authored by Samantha Ming's avatar Samantha Ming Committed by Phil Hughes

Show more context in unresolved jump button

Instead of scrolling to the discussion,
when the user click on the "Jump to next unresolved" button,
it will a fixed amount of extra top line thread.
parent 48ee4c5e
...@@ -244,22 +244,28 @@ export const contentTop = () => { ...@@ -244,22 +244,28 @@ export const contentTop = () => {
); );
}; };
export const scrollToElement = element => { export const scrollToElement = (element, options = {}) => {
let $el = element; let $el = element;
if (!(element instanceof $)) { if (!(element instanceof $)) {
$el = $(element); $el = $(element);
} }
const { top } = $el.offset(); const { top } = $el.offset();
const { offset = 0 } = options;
// eslint-disable-next-line no-jquery/no-animate // eslint-disable-next-line no-jquery/no-animate
return $('body, html').animate( return $('body, html').animate(
{ {
scrollTop: top - contentTop(), scrollTop: top - contentTop() + offset,
}, },
200, 200,
); );
}; };
export const scrollToElementWithContext = element => {
const offsetMultiplier = -0.1;
return scrollToElement(element, { offset: window.innerHeight * offsetMultiplier });
};
/** /**
* Returns a function that can only be invoked once between * Returns a function that can only be invoked once between
* each browser screen repaint. * each browser screen repaint.
......
import { mapGetters, mapActions, mapState } from 'vuex'; import { mapGetters, mapActions, mapState } from 'vuex';
import { scrollToElement } from '~/lib/utils/common_utils'; import { scrollToElementWithContext } from '~/lib/utils/common_utils';
import eventHub from '../event_hub'; import eventHub from '../event_hub';
/** /**
...@@ -10,7 +10,7 @@ function scrollTo(selector) { ...@@ -10,7 +10,7 @@ function scrollTo(selector) {
const el = document.querySelector(selector); const el = document.querySelector(selector);
if (el) { if (el) {
scrollToElement(el); scrollToElementWithContext(el);
return true; return true;
} }
......
---
title: Show more context in unresolved jump button
merge_request: 32737
author:
type: changed
import * as commonUtils from '~/lib/utils/common_utils'; import * as commonUtils from '~/lib/utils/common_utils';
import $ from 'jquery';
describe('common_utils', () => { describe('common_utils', () => {
describe('parseUrl', () => { describe('parseUrl', () => {
...@@ -211,6 +212,59 @@ describe('common_utils', () => { ...@@ -211,6 +212,59 @@ describe('common_utils', () => {
}); });
}); });
describe('scrollToElement*', () => {
let elem;
const windowHeight = 1000;
const elemTop = 100;
beforeEach(() => {
elem = document.createElement('div');
window.innerHeight = windowHeight;
jest.spyOn($.fn, 'animate');
jest.spyOn($.fn, 'offset').mockReturnValue({ top: elemTop });
});
afterEach(() => {
$.fn.animate.mockRestore();
$.fn.offset.mockRestore();
});
describe('scrollToElement', () => {
it('scrolls to element', () => {
commonUtils.scrollToElement(elem);
expect($.fn.animate).toHaveBeenCalledWith(
{
scrollTop: elemTop,
},
expect.any(Number),
);
});
it('scrolls to element with offset', () => {
const offset = 50;
commonUtils.scrollToElement(elem, { offset });
expect($.fn.animate).toHaveBeenCalledWith(
{
scrollTop: elemTop + offset,
},
expect.any(Number),
);
});
});
describe('scrollToElementWithContext', () => {
it('scrolls with context', () => {
commonUtils.scrollToElementWithContext();
expect($.fn.animate).toHaveBeenCalledWith(
{
scrollTop: elemTop - windowHeight * 0.1,
},
expect.any(Number),
);
});
});
});
describe('debounceByAnimationFrame', () => { describe('debounceByAnimationFrame', () => {
it('debounces a function to allow a maximum of one call per animation frame', done => { it('debounces a function to allow a maximum of one call per animation frame', done => {
const spy = jest.fn(); const spy = jest.fn();
......
...@@ -41,7 +41,7 @@ describe('Discussion navigation mixin', () => { ...@@ -41,7 +41,7 @@ describe('Discussion navigation mixin', () => {
.join(''), .join(''),
); );
jest.spyOn(utils, 'scrollToElement'); jest.spyOn(utils, 'scrollToElementWithContext');
expandDiscussion = jest.fn(); expandDiscussion = jest.fn();
const { actions, ...notesRest } = notesModule(); const { actions, ...notesRest } = notesModule();
...@@ -102,7 +102,7 @@ describe('Discussion navigation mixin', () => { ...@@ -102,7 +102,7 @@ describe('Discussion navigation mixin', () => {
}); });
it('scrolls to element', () => { it('scrolls to element', () => {
expect(utils.scrollToElement).toHaveBeenCalledWith( expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
findDiscussion('div.discussion', expected), findDiscussion('div.discussion', expected),
); );
}); });
...@@ -123,11 +123,13 @@ describe('Discussion navigation mixin', () => { ...@@ -123,11 +123,13 @@ describe('Discussion navigation mixin', () => {
}); });
it('scrolls when scrollToDiscussion is emitted', () => { it('scrolls when scrollToDiscussion is emitted', () => {
expect(utils.scrollToElement).not.toHaveBeenCalled(); expect(utils.scrollToElementWithContext).not.toHaveBeenCalled();
eventHub.$emit('scrollToDiscussion'); eventHub.$emit('scrollToDiscussion');
expect(utils.scrollToElement).toHaveBeenCalledWith(findDiscussion('ul.notes', expected)); expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
findDiscussion('ul.notes', expected),
);
}); });
}); });
...@@ -167,7 +169,7 @@ describe('Discussion navigation mixin', () => { ...@@ -167,7 +169,7 @@ describe('Discussion navigation mixin', () => {
}); });
it('scrolls to discussion', () => { it('scrolls to discussion', () => {
expect(utils.scrollToElement).toHaveBeenCalledWith( expect(utils.scrollToElementWithContext).toHaveBeenCalledWith(
findDiscussion('div.discussion', expected), findDiscussion('div.discussion', expected),
); );
}); });
......
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