diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-30 12:10:10 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-12-30 12:10:10 +0300 |
commit | b6f8356ae89345ab8c49b38496065c9f653c1446 (patch) | |
tree | 0e53c9abf1c0168b0a2c62beebdbbd819abb69ca | |
parent | a5b1631549141790e1d74cd6858996a8b8e9b7e0 (diff) |
Add latest changes from gitlab-org/gitlab@master
22 files changed, 186 insertions, 349 deletions
diff --git a/app/assets/javascripts/groups/components/app.vue b/app/assets/javascripts/groups/components/app.vue index ac36d284384..4f26bab8bd3 100644 --- a/app/assets/javascripts/groups/components/app.vue +++ b/app/assets/javascripts/groups/components/app.vue @@ -1,8 +1,6 @@ <script> /* global Flash */ -import $ from 'jquery'; -import 'vendor/jquery.scrollTo'; import { GlLoadingIcon, GlModal } from '@gitlab/ui'; import { __, s__, sprintf } from '~/locale'; import { HIDDEN_CLASS } from '~/lib/utils/constants'; @@ -116,7 +114,7 @@ export default { }) .catch(() => { this.isLoading = false; - $.scrollTo(0); + window.scrollTo({ top: 0, behavior: 'smooth' }); Flash(COMMON_STR.FAILURE); }); @@ -151,7 +149,7 @@ export default { updatePagination: true, }).then((res) => { this.isLoading = false; - $.scrollTo(0); + window.scrollTo({ top: 0, behavior: 'smooth' }); const currentPath = mergeUrlParams({ page }, window.location.href); window.history.replaceState( @@ -195,7 +193,7 @@ export default { this.service .leaveGroup(this.targetGroup.leavePath) .then((res) => { - $.scrollTo(0); + window.scrollTo({ top: 0, behavior: 'smooth' }); this.store.removeGroup(this.targetGroup, this.targetParentGroup); this.$toast.show(res.data.notice); }) diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index ba1280d89d5..1c8cb423828 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -4,7 +4,7 @@ import { GlBreakpointInstance as breakpointInstance } from '@gitlab/ui/dist/utils'; import $ from 'jquery'; -import { isFunction } from 'lodash'; +import { isFunction, defer } from 'lodash'; import Cookies from 'js-cookie'; import axios from './axios_utils'; import { getLocationHash } from './url_utility'; @@ -261,20 +261,23 @@ export const contentTop = () => { }; export const scrollToElement = (element, options = {}) => { - let $el = element; - if (!(element instanceof $)) { - $el = $(element); + let el = element; + if (element instanceof $) { + // eslint-disable-next-line prefer-destructuring + el = element[0]; + } else if (typeof el === 'string') { + el = document.querySelector(element); } - const { top } = $el.offset(); - const { offset = 0 } = options; - // eslint-disable-next-line no-jquery/no-animate - return $('body, html').animate( - { - scrollTop: top - contentTop() + offset, - }, - 200, - ); + if (el && el.getBoundingClientRect) { + // In the previous implementation, jQuery naturally deferred this scrolling. + // Unfortunately, we're quite coupled to this implementation detail now. + defer(() => { + const { duration = 200, offset = 0 } = options; + const y = el.getBoundingClientRect().top + window.pageYOffset + offset - contentTop(); + window.scrollTo({ top: y, behavior: duration ? 'smooth' : 'auto' }); + }); + } }; export const scrollToElementWithContext = (element) => { diff --git a/app/assets/javascripts/line_highlighter.js b/app/assets/javascripts/line_highlighter.js index 314ade0dc3e..aaa8ee40966 100644 --- a/app/assets/javascripts/line_highlighter.js +++ b/app/assets/javascripts/line_highlighter.js @@ -1,7 +1,7 @@ /* eslint-disable func-names, no-underscore-dangle, no-param-reassign, consistent-return */ import $ from 'jquery'; -import 'vendor/jquery.scrollTo'; +import { scrollToElement } from '~/lib/utils/common_utils'; // LineHighlighter // @@ -69,16 +69,12 @@ LineHighlighter.prototype.highlightHash = function (newHash) { if (range[0]) { this.highlightRange(range); const lineSelector = `#L${range[0]}`; - const scrollOptions = { + + scrollToElement(lineSelector, { // Scroll to the first highlighted line on initial load - // Offset -50 for the sticky top bar, and another -100 for some context - offset: -150, - }; - if (this.options.scrollFileHolder) { - $(this.options.fileHolderSelector).scrollTo(lineSelector, scrollOptions); - } else { - $.scrollTo(lineSelector, scrollOptions); - } + // Add an offset of -100 for some context + offset: -100, + }); } } }; diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 071dd62ad52..81241cd2418 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -1,7 +1,6 @@ /* eslint-disable no-new, class-methods-use-this */ import $ from 'jquery'; -import 'vendor/jquery.scrollTo'; import { GlBreakpointInstance as bp } from '@gitlab/ui/dist/utils'; import Cookies from 'js-cookie'; import createEventHub from '~/helpers/event_hub_factory'; @@ -14,6 +13,7 @@ import { handleLocationHash, isMetaClick, parseBoolean, + scrollToElement, } from './lib/utils/common_utils'; import { isInVueNoteablePage } from './lib/utils/dom_utils'; import { getLocationHash } from './lib/utils/url_utility'; @@ -255,12 +255,12 @@ export default class MergeRequestTabs { this.eventHub.$emit('MergeRequestTabChange', action); } - scrollToElement(container) { + scrollToContainerElement(container) { if (location.hash) { - const offset = 0 - ($('.navbar-gitlab').outerHeight() + $('.js-tabs-affix').outerHeight()); const $el = $(`${container} ${location.hash}:not(.match)`); + if ($el.length) { - $.scrollTo($el[0], { offset }); + scrollToElement($el[0]); } } } @@ -339,7 +339,7 @@ export default class MergeRequestTabs { document.querySelector('div#commits').innerHTML = data.html; localTimeAgo($('.js-timeago', 'div#commits')); this.commitsLoaded = true; - this.scrollToElement('#commits'); + this.scrollToContainerElement('#commits'); this.toggleLoading(false); initAddContextCommitsTriggers(); @@ -408,7 +408,7 @@ export default class MergeRequestTabs { this.diffsLoaded = true; new Diff(); - this.scrollToElement('#diffs'); + this.scrollToContainerElement('#diffs'); $('.diff-file').each((i, el) => { new BlobForkSuggestion({ diff --git a/app/assets/javascripts/zen_mode.js b/app/assets/javascripts/zen_mode.js index 3ee9a19e2d9..06ba2496a99 100644 --- a/app/assets/javascripts/zen_mode.js +++ b/app/assets/javascripts/zen_mode.js @@ -1,4 +1,4 @@ -/* eslint-disable consistent-return, class-methods-use-this */ +/* eslint-disable consistent-return */ // Zen Mode (full screen) textarea // @@ -6,10 +6,10 @@ /*= provides zen_mode:leave */ import $ from 'jquery'; -import 'vendor/jquery.scrollTo'; import Dropzone from 'dropzone'; import Mousetrap from 'mousetrap'; import 'mousetrap/plugins/pause/mousetrap-pause'; +import { scrollToElement } from '~/lib/utils/common_utils'; Dropzone.autoDiscover = false; @@ -76,7 +76,7 @@ export default class ZenMode { if (this.active_textarea) { Mousetrap.unpause(); this.active_textarea.closest('.zen-backdrop').removeClass('fullscreen'); - this.scrollTo(this.active_textarea); + scrollToElement(this.active_textarea, { duration: 0, offset: -100 }); this.active_textarea = null; this.active_backdrop = null; @@ -86,10 +86,4 @@ export default class ZenMode { } } } - - scrollTo(zenArea) { - return $.scrollTo(zenArea, 0, { - offset: -150, - }); - } } diff --git a/changelogs/unreleased/293679_introduce_dismissal_reason_for_vulnerability_feedback.yml b/changelogs/unreleased/293679_introduce_dismissal_reason_for_vulnerability_feedback.yml new file mode 100644 index 00000000000..edb39b80600 --- /dev/null +++ b/changelogs/unreleased/293679_introduce_dismissal_reason_for_vulnerability_feedback.yml @@ -0,0 +1,5 @@ +--- +title: Add `dismissal_reason` column into the `vulnerability_feedback` table +merge_request: 50632 +author: +type: added diff --git a/db/migrate/20201228184500_add_dismissal_reason_into_vulnerability_feedback_table.rb b/db/migrate/20201228184500_add_dismissal_reason_into_vulnerability_feedback_table.rb new file mode 100644 index 00000000000..92484aced4e --- /dev/null +++ b/db/migrate/20201228184500_add_dismissal_reason_into_vulnerability_feedback_table.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class AddDismissalReasonIntoVulnerabilityFeedbackTable < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def up + add_column :vulnerability_feedback, :dismissal_reason, :smallint + end + + def down + remove_column :vulnerability_feedback, :dismissal_reason + end +end diff --git a/db/schema_migrations/20201228184500 b/db/schema_migrations/20201228184500 new file mode 100644 index 00000000000..c6168e775b2 --- /dev/null +++ b/db/schema_migrations/20201228184500 @@ -0,0 +1 @@ +9018fed4aab19642fafee3e50bf41be422fc3f8256d0b5d78c8a70fc96f4090f
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 097006e024c..a9022190aeb 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -17645,7 +17645,8 @@ CREATE TABLE vulnerability_feedback ( comment_author_id integer, comment text, comment_timestamp timestamp with time zone, - finding_uuid uuid + finding_uuid uuid, + dismissal_reason smallint ); CREATE SEQUENCE vulnerability_feedback_id_seq diff --git a/qa/qa/flow/sign_up.rb b/qa/qa/flow/sign_up.rb index 8fb93e74ec0..ff1f411277b 100644 --- a/qa/qa/flow/sign_up.rb +++ b/qa/qa/flow/sign_up.rb @@ -6,36 +6,34 @@ module QA module_function def sign_up!(user) + Page::Main::Menu.perform(&:sign_out_if_signed_in) Page::Main::Login.perform(&:switch_to_register_page) + Page::Registration::SignUp.perform do |sign_up| + sign_up.fill_new_user_first_name_field(user.first_name) + sign_up.fill_new_user_last_name_field(user.last_name) + sign_up.fill_new_user_username_field(user.username) + sign_up.fill_new_user_email_field(user.email) + sign_up.fill_new_user_password_field(user.password) + + # Because invisible_captcha would prevent submitting this form + # within 4 seconds, sleep here. This can be removed once we + # implement invisible_captcha as an application setting instead + # of a feature flag, so we can turn it off while testing. + # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/284113 + sleep 5 + + sign_up.click_new_user_register_button + end - success = Support::Retrier.retry_until(raise_on_failure: false) do - Page::Registration::SignUp.perform do |sign_up| - sign_up.fill_new_user_first_name_field(user.first_name) - sign_up.fill_new_user_last_name_field(user.last_name) - sign_up.fill_new_user_username_field(user.username) - sign_up.fill_new_user_email_field(user.email) - sign_up.fill_new_user_password_field(user.password) - - # Because invisible_captcha would prevent submitting this form - # within 4 seconds, sleep here. This can be removed once we - # implement invisible_captcha as an application setting instead - # of a feature flag, so we can turn it off while testing. - # Issue: https://gitlab.com/gitlab-org/gitlab/-/issues/284113 - sleep 5 - - sign_up.click_new_user_register_button - end - - Page::Registration::Welcome.perform(&:click_get_started_button_if_available) + Page::Registration::Welcome.perform(&:click_get_started_button_if_available) - if user.expect_fabrication_success - Page::Main::Menu.perform(&:has_personal_area?) - else - Page::Main::Menu.perform(&:not_signed_in?) - end + success = if user.expect_fabrication_success + Page::Main::Menu.perform(&:has_personal_area?) + else + Page::Main::Menu.perform(&:not_signed_in?) end - raise "Failed to register the user" unless success + raise "Failed user registration attempt. Registration was expected to #{ user.expect_fabrication_success ? 'succeed' : 'fail'} but #{success ? 'succeeded' : 'failed'}." unless success end def disable_sign_ups diff --git a/qa/qa/page/base.rb b/qa/qa/page/base.rb index 00b48dc7fe9..763150bb6b3 100644 --- a/qa/qa/page/base.rb +++ b/qa/qa/page/base.rb @@ -135,8 +135,11 @@ module QA def check_element(name) retry_until(sleep_interval: 1) do find_element(name).set(true) + checked = find_element(name).checked? - find_element(name).checked? + QA::Runtime::Logger.debug(checked ? "#{name} was checked" : "#{name} was not checked") + + checked end end diff --git a/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb index d58857f6da2..d9b246fc458 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/login/register_spec.rb @@ -98,8 +98,10 @@ module QA before do enable_require_admin_approval_after_user_signup_via_ui - @user = Resource::User.fabricate_via_browser_ui! do |user| - user.expect_fabrication_success = false + Support::Retrier.retry_on_exception do + @user = Resource::User.fabricate_via_browser_ui! do |user| + user.expect_fabrication_success = false + end end end @@ -148,26 +150,34 @@ module QA end def set_require_admin_approval_after_user_signup_via_api(enable_or_disable) - return if Runtime::ApplicationSettings.get_application_settings[:require_admin_approval_after_user_signup] == enable_or_disable + return if get_require_admin_approval_after_user_signup_via_api == enable_or_disable Runtime::ApplicationSettings.set_application_settings(require_admin_approval_after_user_signup: enable_or_disable) sleep 10 # It takes a moment for the setting to come into effect end + def get_require_admin_approval_after_user_signup_via_api + Runtime::ApplicationSettings.get_application_settings[:require_admin_approval_after_user_signup] + end + def enable_require_admin_approval_after_user_signup_via_ui - unless Runtime::ApplicationSettings.get_application_settings[:require_admin_approval_after_user_signup] - Flow::Login.while_signed_in_as_admin do - Page::Main::Menu.perform(&:go_to_admin_area) - QA::Page::Admin::Menu.perform(&:go_to_general_settings) - Page::Admin::Settings::General.perform do |setting| - setting.expand_sign_up_restrictions do |settings| - settings.require_admin_approval_after_user_signup + unless get_require_admin_approval_after_user_signup_via_api + QA::Support::Retrier.retry_until do + Flow::Login.while_signed_in_as_admin do + Page::Main::Menu.perform(&:go_to_admin_area) + QA::Page::Admin::Menu.perform(&:go_to_general_settings) + Page::Admin::Settings::General.perform do |setting| + setting.expand_sign_up_restrictions do |settings| + settings.require_admin_approval_after_user_signup + end end end - end - sleep 10 # It takes a moment for the setting to come into effect + sleep 15 # It takes a moment for the setting to come into effect + + get_require_admin_approval_after_user_signup_via_api + end end end end diff --git a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb index cb7c952dfe4..b86586d53e2 100644 --- a/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb +++ b/spec/features/merge_request/user_resolves_diff_notes_and_discussions_resolve_spec.rb @@ -228,6 +228,7 @@ RSpec.describe 'Merge request > User resolves diff notes and threads', :js do page.find('.discussion-next-btn').click end + expect(page).to have_button('Resolve thread', visible: true) expect(page.evaluate_script("window.pageYOffset")).to be > 0 end diff --git a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb index d9950f5504b..56517a97716 100644 --- a/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb +++ b/spec/features/merge_request/user_scrolls_to_note_on_load_spec.rb @@ -17,15 +17,16 @@ RSpec.describe 'Merge request > User scrolls to note on load', :js do it 'scrolls note into view' do visit "#{project_merge_request_path(project, merge_request)}#{fragment_id}" - wait_for_requests + wait_for_all_requests + + expect(page).to have_selector("#{fragment_id}") - page_height = page.current_window.size[1] page_scroll_y = page.evaluate_script("window.scrollY") - fragment_position_top = page.evaluate_script("Math.round($('#{fragment_id}').offset().top)") + fragment_position_top = page.evaluate_script("Math.round(document.querySelector('#{fragment_id}').getBoundingClientRect().top + window.pageYOffset)") expect(find(fragment_id).visible?).to eq true expect(fragment_position_top).to be >= page_scroll_y - expect(fragment_position_top).to be < (page_scroll_y + page_height) + expect(page.evaluate_script("window.pageYOffset")).to be > 0 end it 'renders un-collapsed notes with diff' do diff --git a/spec/frontend/__mocks__/lodash/defer.js b/spec/frontend/__mocks__/lodash/defer.js new file mode 100644 index 00000000000..9325cb28ba6 --- /dev/null +++ b/spec/frontend/__mocks__/lodash/defer.js @@ -0,0 +1,5 @@ +/** + * Instead of messing around with timers, we execute deferred functions + * immediately in our specs + */ +export default (fn, ...args) => fn(...args); diff --git a/spec/frontend/groups/components/app_spec.js b/spec/frontend/groups/components/app_spec.js index 72d8e23f28b..9244e4f331e 100644 --- a/spec/frontend/groups/components/app_spec.js +++ b/spec/frontend/groups/components/app_spec.js @@ -1,5 +1,4 @@ import '~/flash'; -import $ from 'jquery'; import Vue from 'vue'; import AxiosMockAdapter from 'axios-mock-adapter'; import { GlModal, GlLoadingIcon } from '@gitlab/ui'; @@ -123,12 +122,12 @@ describe('AppComponent', () => { it('should show flash error when request fails', () => { mock.onGet('/dashboard/groups.json').reply(400); - jest.spyOn($, 'scrollTo').mockImplementation(() => {}); + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); jest.spyOn(window, 'Flash').mockImplementation(() => {}); return vm.fetchGroups({}).then(() => { expect(vm.isLoading).toBe(false); - expect($.scrollTo).toHaveBeenCalledWith(0); + expect(window.scrollTo).toHaveBeenCalledWith({ behavior: 'smooth', top: 0 }); expect(window.Flash).toHaveBeenCalledWith('An error occurred. Please try again.'); }); }); @@ -180,7 +179,7 @@ describe('AppComponent', () => { it('should fetch groups for provided page details and update window state', () => { jest.spyOn(urlUtilities, 'mergeUrlParams'); jest.spyOn(window.history, 'replaceState').mockImplementation(() => {}); - jest.spyOn($, 'scrollTo').mockImplementation(() => {}); + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); const fetchPagePromise = vm.fetchPage(2, null, null, true); @@ -195,7 +194,7 @@ describe('AppComponent', () => { return fetchPagePromise.then(() => { expect(vm.isLoading).toBe(false); - expect($.scrollTo).toHaveBeenCalledWith(0); + expect(window.scrollTo).toHaveBeenCalledWith({ behavior: 'smooth', top: 0 }); expect(urlUtilities.mergeUrlParams).toHaveBeenCalledWith({ page: 2 }, expect.any(String)); expect(window.history.replaceState).toHaveBeenCalledWith( { @@ -308,14 +307,14 @@ describe('AppComponent', () => { const notice = `You left the "${childGroupItem.fullName}" group.`; jest.spyOn(vm.service, 'leaveGroup').mockResolvedValue({ data: { notice } }); jest.spyOn(vm.store, 'removeGroup'); - jest.spyOn($, 'scrollTo').mockImplementation(() => {}); + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); vm.leaveGroup(); expect(vm.targetGroup.isBeingRemoved).toBe(true); expect(vm.service.leaveGroup).toHaveBeenCalledWith(vm.targetGroup.leavePath); return waitForPromises().then(() => { - expect($.scrollTo).toHaveBeenCalledWith(0); + expect(window.scrollTo).toHaveBeenCalledWith({ behavior: 'smooth', top: 0 }); expect(vm.store.removeGroup).toHaveBeenCalledWith(vm.targetGroup, vm.targetParentGroup); expect($toast.show).toHaveBeenCalledWith(notice); }); diff --git a/spec/frontend/ide/stores/actions/tree_spec.js b/spec/frontend/ide/stores/actions/tree_spec.js index ece10d2a602..8322e401a90 100644 --- a/spec/frontend/ide/stores/actions/tree_spec.js +++ b/spec/frontend/ide/stores/actions/tree_spec.js @@ -57,26 +57,15 @@ describe('Multi-file store tree actions', () => { }); it('calls service getFiles', () => { - return ( - store - .dispatch('getFiles', basicCallParameters) - // getFiles actions calls lodash.defer - .then(() => jest.runOnlyPendingTimers()) - .then(() => { - expect(service.getFiles).toHaveBeenCalledWith('foo/abcproject', '12345678'); - }) - ); + return store.dispatch('getFiles', basicCallParameters).then(() => { + expect(service.getFiles).toHaveBeenCalledWith('foo/abcproject', '12345678'); + }); }); it('adds data into tree', (done) => { store .dispatch('getFiles', basicCallParameters) .then(() => { - // The populating of the tree is deferred for performance reasons. - // See this merge request for details: https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/25700 - jest.advanceTimersByTime(1); - }) - .then(() => { projectTree = store.state.trees['abcproject/master']; expect(projectTree.tree.length).toBe(2); diff --git a/spec/frontend/lib/utils/common_utils_spec.js b/spec/frontend/lib/utils/common_utils_spec.js index ddb0495fd1f..84aa9c32288 100644 --- a/spec/frontend/lib/utils/common_utils_spec.js +++ b/spec/frontend/lib/utils/common_utils_spec.js @@ -1,4 +1,3 @@ -import $ from 'jquery'; import * as commonUtils from '~/lib/utils/common_utils'; describe('common_utils', () => { @@ -214,54 +213,84 @@ describe('common_utils', () => { describe('scrollToElement*', () => { let elem; - const windowHeight = 1000; + const windowHeight = 550; const elemTop = 100; + const id = 'scroll_test'; beforeEach(() => { elem = document.createElement('div'); + elem.id = id; + document.body.appendChild(elem); window.innerHeight = windowHeight; window.mrTabs = { currentAction: 'show' }; - jest.spyOn($.fn, 'animate'); - jest.spyOn($.fn, 'offset').mockReturnValue({ top: elemTop }); + jest.spyOn(window, 'scrollTo').mockImplementation(() => {}); + jest.spyOn(Element.prototype, 'getBoundingClientRect').mockReturnValue({ top: elemTop }); }); afterEach(() => { - $.fn.animate.mockRestore(); - $.fn.offset.mockRestore(); + window.scrollTo.mockRestore(); + Element.prototype.getBoundingClientRect.mockRestore(); + elem.remove(); }); - describe('scrollToElement', () => { + describe('scrollToElement with HTMLElement', () => { it('scrolls to element', () => { commonUtils.scrollToElement(elem); - expect($.fn.animate).toHaveBeenCalledWith( - { - scrollTop: elemTop, - }, - expect.any(Number), - ); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elemTop, + }); }); it('scrolls to element with offset', () => { const offset = 50; commonUtils.scrollToElement(elem, { offset }); - expect($.fn.animate).toHaveBeenCalledWith( - { - scrollTop: elemTop + offset, - }, - expect.any(Number), - ); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elemTop + offset, + }); + }); + }); + + describe('scrollToElement with Selector', () => { + it('scrolls to element', () => { + commonUtils.scrollToElement(`#${id}`); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elemTop, + }); + }); + + it('scrolls to element with offset', () => { + const offset = 50; + commonUtils.scrollToElement(`#${id}`, { offset }); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elemTop + offset, + }); }); }); describe('scrollToElementWithContext', () => { - it('scrolls with context', () => { - commonUtils.scrollToElementWithContext(); - expect($.fn.animate).toHaveBeenCalledWith( - { - scrollTop: elemTop - windowHeight * 0.1, - }, - expect.any(Number), - ); + // This is what the implementation of scrollToElementWithContext + // scrolls to, in case we change tha implementation + // it needs to be adjusted + const elementTopWithContext = elemTop - windowHeight * 0.1; + + it('with HTMLElement scrolls with context', () => { + commonUtils.scrollToElementWithContext(elem); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elementTopWithContext, + }); + }); + + it('with Selector scrolls with context', () => { + commonUtils.scrollToElementWithContext(`#${id}`); + expect(window.scrollTo).toHaveBeenCalledWith({ + behavior: 'smooth', + top: elementTopWithContext, + }); }); }); }); diff --git a/spec/frontend/line_highlighter_spec.js b/spec/frontend/line_highlighter_spec.js index dd4f85deccd..45d6f02ed2c 100644 --- a/spec/frontend/line_highlighter_spec.js +++ b/spec/frontend/line_highlighter_spec.js @@ -2,6 +2,7 @@ import $ from 'jquery'; import LineHighlighter from '~/line_highlighter'; +import * as utils from '~/lib/utils/common_utils'; describe('LineHighlighter', () => { const testContext = {}; @@ -50,10 +51,10 @@ describe('LineHighlighter', () => { }); it('scrolls to the first highlighted line on initial load', () => { - const spy = jest.spyOn($, 'scrollTo'); + jest.spyOn(utils, 'scrollToElement'); new LineHighlighter({ hash: '#L5-25' }); - expect(spy).toHaveBeenCalledWith('#L5', expect.anything()); + expect(utils.scrollToElement).toHaveBeenCalledWith('#L5', expect.anything()); }); it('discards click events', () => { diff --git a/spec/frontend/merge_request_tabs_spec.js b/spec/frontend/merge_request_tabs_spec.js index 665e92cf2db..82462036219 100644 --- a/spec/frontend/merge_request_tabs_spec.js +++ b/spec/frontend/merge_request_tabs_spec.js @@ -5,7 +5,6 @@ import axios from '~/lib/utils/axios_utils'; import MergeRequestTabs from '~/merge_request_tabs'; import '~/commit/pipelines/pipelines_bundle'; import '~/lib/utils/common_utils'; -import 'vendor/jquery.scrollTo'; jest.mock('~/lib/utils/webpack', () => ({ resetServiceWorkersPublicPath: jest.fn(), diff --git a/spec/frontend/zen_mode_spec.js b/spec/frontend/zen_mode_spec.js index 8e0d170289b..e874d018e92 100644 --- a/spec/frontend/zen_mode_spec.js +++ b/spec/frontend/zen_mode_spec.js @@ -3,6 +3,7 @@ import axios from 'axios'; import MockAdapter from 'axios-mock-adapter'; import Dropzone from 'dropzone'; import Mousetrap from 'mousetrap'; +import * as utils from '~/lib/utils/common_utils'; import ZenMode from '~/zen_mode'; import initNotes from '~/init_notes'; @@ -103,10 +104,10 @@ describe('ZenMode', () => { }); it('restores the scroll position', () => { - jest.spyOn(zen, 'scrollTo').mockImplementation(() => {}); + jest.spyOn(utils, 'scrollToElement'); exitZen(); - expect(zen.scrollTo).toHaveBeenCalled(); + expect(utils.scrollToElement).toHaveBeenCalled(); }); }); }); diff --git a/vendor/assets/javascripts/jquery.scrollTo.js b/vendor/assets/javascripts/jquery.scrollTo.js deleted file mode 100644 index 7ba17766b70..00000000000 --- a/vendor/assets/javascripts/jquery.scrollTo.js +++ /dev/null @@ -1,210 +0,0 @@ -/*! - * jQuery.scrollTo - * Copyright (c) 2007-2015 Ariel Flesler - aflesler<a>gmail<d>com | http://flesler.blogspot.com - * Licensed under MIT - * http://flesler.blogspot.com/2007/10/jqueryscrollto.html - * @projectDescription Lightweight, cross-browser and highly customizable animated scrolling with jQuery - * @author Ariel Flesler - * @version 2.1.2 - */ -;(function(factory) { - 'use strict'; - if (typeof define === 'function' && define.amd) { - // AMD - define(['jquery'], factory); - } else if (typeof module !== 'undefined' && module.exports) { - // CommonJS - module.exports = factory(require('jquery')); - } else { - // Global - factory(jQuery); - } -})(function($) { - 'use strict'; - - var $scrollTo = $.scrollTo = function(target, duration, settings) { - return $(window).scrollTo(target, duration, settings); - }; - - $scrollTo.defaults = { - axis:'xy', - duration: 0, - limit:true - }; - - function isWin(elem) { - return !elem.nodeName || - $.inArray(elem.nodeName.toLowerCase(), ['iframe','#document','html','body']) !== -1; - } - - $.fn.scrollTo = function(target, duration, settings) { - if (typeof duration === 'object') { - settings = duration; - duration = 0; - } - if (typeof settings === 'function') { - settings = { onAfter:settings }; - } - if (target === 'max') { - target = 9e9; - } - - settings = $.extend({}, $scrollTo.defaults, settings); - // Speed is still recognized for backwards compatibility - duration = duration || settings.duration; - // Make sure the settings are given right - var queue = settings.queue && settings.axis.length > 1; - if (queue) { - // Let's keep the overall duration - duration /= 2; - } - settings.offset = both(settings.offset); - settings.over = both(settings.over); - - return this.each(function() { - // Null target yields nothing, just like jQuery does - if (target === null) return; - - var win = isWin(this), - elem = win ? this.contentWindow || window : this, - $elem = $(elem), - targ = target, - attr = {}, - toff; - - switch (typeof targ) { - // A number will pass the regex - case 'number': - case 'string': - if (/^([+-]=?)?\d+(\.\d+)?(px|%)?$/.test(targ)) { - targ = both(targ); - // We are done - break; - } - // Relative/Absolute selector - targ = win ? $(targ) : $(targ, elem); - /* falls through */ - case 'object': - if (targ.length === 0) return; - // DOMElement / jQuery - if (targ.is || targ.style) { - // Get the real position of the target - toff = (targ = $(targ)).offset(); - } - } - - var offset = $.isFunction(settings.offset) && settings.offset(elem, targ) || settings.offset; - - $.each(settings.axis.split(''), function(i, axis) { - var Pos = axis === 'x' ? 'Left' : 'Top', - pos = Pos.toLowerCase(), - key = 'scroll' + Pos, - prev = $elem[key](), - max = $scrollTo.max(elem, axis); - - if (toff) {// jQuery / DOMElement - attr[key] = toff[pos] + (win ? 0 : prev - $elem.offset()[pos]); - - // If it's a dom element, reduce the margin - if (settings.margin) { - attr[key] -= parseInt(targ.css('margin'+Pos), 10) || 0; - attr[key] -= parseInt(targ.css('border'+Pos+'Width'), 10) || 0; - } - - attr[key] += offset[pos] || 0; - - if (settings.over[pos]) { - // Scroll to a fraction of its width/height - attr[key] += targ[axis === 'x'?'width':'height']() * settings.over[pos]; - } - } else { - var val = targ[pos]; - // Handle percentage values - attr[key] = val.slice && val.slice(-1) === '%' ? - parseFloat(val) / 100 * max - : val; - } - - // Number or 'number' - if (settings.limit && /^\d+$/.test(attr[key])) { - // Check the limits - attr[key] = attr[key] <= 0 ? 0 : Math.min(attr[key], max); - } - - // Don't waste time animating, if there's no need. - if (!i && settings.axis.length > 1) { - if (prev === attr[key]) { - // No animation needed - attr = {}; - } else if (queue) { - // Intermediate animation - animate(settings.onAfterFirst); - // Don't animate this axis again in the next iteration. - attr = {}; - } - } - }); - - animate(settings.onAfter); - - function animate(callback) { - var opts = $.extend({}, settings, { - // The queue setting conflicts with animate() - // Force it to always be true - queue: true, - duration: duration, - complete: callback && function() { - callback.call(elem, targ, settings); - } - }); - $elem.animate(attr, opts); - } - }); - }; - - // Max scrolling position, works on quirks mode - // It only fails (not too badly) on IE, quirks mode. - $scrollTo.max = function(elem, axis) { - var Dim = axis === 'x' ? 'Width' : 'Height', - scroll = 'scroll'+Dim; - - if (!isWin(elem)) - return elem[scroll] - $(elem)[Dim.toLowerCase()](); - - var size = 'client' + Dim, - doc = elem.ownerDocument || elem.document, - html = doc.documentElement, - body = doc.body; - - return Math.max(html[scroll], body[scroll]) - Math.min(html[size], body[size]); - }; - - function both(val) { - return $.isFunction(val) || $.isPlainObject(val) ? val : { top:val, left:val }; - } - - // Add special hooks so that window scroll properties can be animated - $.Tween.propHooks.scrollLeft = - $.Tween.propHooks.scrollTop = { - get: function(t) { - return $(t.elem)[t.prop](); - }, - set: function(t) { - var curr = this.get(t); - // If interrupt is true and user scrolled, stop animating - if (t.options.interrupt && t._last && t._last !== curr) { - return $(t.elem).stop(); - } - var next = Math.round(t.now); - // Don't waste CPU - // Browsers don't render floating point scroll - if (curr !== next) { - $(t.elem)[t.prop](next); - t._last = this.get(t); - } - } - }; - - // AMD requirement - return $scrollTo; -}); |