From 67fd675bf38451a55f3d6ea8e42295ca9d6315ef Mon Sep 17 00:00:00 2001 From: Graham Lewis <44037625+gclssvglx@users.noreply.github.com> Date: Mon, 25 Oct 2021 17:15:05 +0100 Subject: [PATCH 1/4] Remove jQuery from javascripts/modules/sticky-element-container.js What This change removes jQuery from the sticky-element-container.js module and replaces it with vanilla JS Why We've removing jQuery from GOV.UK --- .../modules/sticky-element-container.js | 31 +++++++++---------- 1 file changed, 15 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/modules/sticky-element-container.js b/app/assets/javascripts/modules/sticky-element-container.js index 970c7fdc5..dc3768140 100644 --- a/app/assets/javascripts/modules/sticky-element-container.js +++ b/app/assets/javascripts/modules/sticky-element-container.js @@ -10,27 +10,26 @@ (function (Modules, root) { 'use strict' - var $ = root.$ - var $window = $(root) - Modules.StickyElementContainer = function () { var self = this self._getWindowDimensions = function _getWindowDimensions () { return { - height: $window.height(), - width: $window.width() + height: root.innerHeight, + width: root.innerWidth } } self._getWindowPositions = function _getWindowPositions () { return { - scrollTop: $window.scrollTop() + scrollTop: root.scrollY } } self.start = function start ($el) { - var $element = $el.find('[data-sticky-element]') + $el = $el[0] + var $element = $el.querySelector('[data-sticky-element]') + var _hasResized = true var _hasScrolled = true var _interval = 50 @@ -40,13 +39,13 @@ initialise() function initialise () { - $window.resize(onResize) - $window.scroll(onScroll) + root.onresize = onResize + root.onscroll = onScroll setInterval(checkResize, _interval) setInterval(checkScroll, _interval) checkResize() checkScroll() - $element.addClass('sticky-element--enabled') + $element.classList.add('sticky-element--enabled') } function onResize () { @@ -63,8 +62,8 @@ _hasScrolled = true var windowDimensions = self._getWindowDimensions() - _startPosition = $el.offset().top - _stopPosition = $el.offset().top + $el.height() - windowDimensions.height + _startPosition = $el.offsetTop + _stopPosition = $el.offsetTop + $el.offsetHeight - windowDimensions.height } } @@ -98,19 +97,19 @@ } function stickToWindow () { - $element.addClass('sticky-element--stuck-to-window') + $element.classList.add('sticky-element--stuck-to-window') } function stickToParent () { - $element.removeClass('sticky-element--stuck-to-window') + $element.classList.remove('sticky-element--stuck-to-window') } function show () { - $element.removeClass('sticky-element--hidden') + $element.classList.remove('sticky-element--hidden') } function hide () { - $element.addClass('sticky-element--hidden') + $element.classList.add('sticky-element--hidden') } } } From 64f5be596379ef8914c2c7b39a492ab0b73e84fa Mon Sep 17 00:00:00 2001 From: Graham Lewis <44037625+gclssvglx@users.noreply.github.com> Date: Wed, 27 Oct 2021 13:39:24 +0100 Subject: [PATCH 2/4] Clean up function and attribute names --- .../modules/sticky-element-container.js | 60 +++++++++---------- .../modules/sticky-element-container.spec.js | 6 +- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/app/assets/javascripts/modules/sticky-element-container.js b/app/assets/javascripts/modules/sticky-element-container.js index dc3768140..cb15df6e4 100644 --- a/app/assets/javascripts/modules/sticky-element-container.js +++ b/app/assets/javascripts/modules/sticky-element-container.js @@ -13,65 +13,65 @@ Modules.StickyElementContainer = function () { var self = this - self._getWindowDimensions = function _getWindowDimensions () { + self.getWindowDimensions = function () { return { height: root.innerHeight, width: root.innerWidth } } - self._getWindowPositions = function _getWindowPositions () { + self.getWindowPositions = function () { return { scrollTop: root.scrollY } } - self.start = function start ($el) { - $el = $el[0] - var $element = $el.querySelector('[data-sticky-element]') + self.start = function ($el) { + var el = $el[0] + var element = el.querySelector('[data-sticky-element]') - var _hasResized = true - var _hasScrolled = true - var _interval = 50 - var _windowVerticalPosition = 1 - var _startPosition, _stopPosition + var hasResized = true + var hasScrolled = true + var interval = 50 + var windowVerticalPosition = 1 + var startPosition, stopPosition initialise() function initialise () { root.onresize = onResize root.onscroll = onScroll - setInterval(checkResize, _interval) - setInterval(checkScroll, _interval) + setInterval(checkResize, interval) + setInterval(checkScroll, interval) checkResize() checkScroll() - $element.classList.add('sticky-element--enabled') + element.classList.add('sticky-element--enabled') } function onResize () { - _hasResized = true + hasResized = true } function onScroll () { - _hasScrolled = true + hasScrolled = true } function checkResize () { - if (_hasResized) { - _hasResized = false - _hasScrolled = true + if (hasResized) { + hasResized = false + hasScrolled = true - var windowDimensions = self._getWindowDimensions() - _startPosition = $el.offsetTop - _stopPosition = $el.offsetTop + $el.offsetHeight - windowDimensions.height + var windowDimensions = self.getWindowDimensions() + startPosition = el.offsetTop + stopPosition = el.offsetTop + el.offsetHeight - windowDimensions.height } } function checkScroll () { - if (_hasScrolled) { - _hasScrolled = false + if (hasScrolled) { + hasScrolled = false - _windowVerticalPosition = self._getWindowPositions().scrollTop + windowVerticalPosition = self.getWindowPositions().scrollTop updateVisibility() updatePosition() @@ -79,7 +79,7 @@ } function updateVisibility () { - var isPastStart = _startPosition < _windowVerticalPosition + var isPastStart = startPosition < windowVerticalPosition if (isPastStart) { show() } else { @@ -88,7 +88,7 @@ } function updatePosition () { - var isPastEnd = _stopPosition < _windowVerticalPosition + var isPastEnd = stopPosition < windowVerticalPosition if (isPastEnd) { stickToParent() } else { @@ -97,19 +97,19 @@ } function stickToWindow () { - $element.classList.add('sticky-element--stuck-to-window') + element.classList.add('sticky-element--stuck-to-window') } function stickToParent () { - $element.classList.remove('sticky-element--stuck-to-window') + element.classList.remove('sticky-element--stuck-to-window') } function show () { - $element.classList.remove('sticky-element--hidden') + element.classList.remove('sticky-element--hidden') } function hide () { - $element.classList.add('sticky-element--hidden') + element.classList.add('sticky-element--hidden') } } } diff --git a/spec/javascripts/modules/sticky-element-container.spec.js b/spec/javascripts/modules/sticky-element-container.spec.js index 894f8cc9b..35356892a 100644 --- a/spec/javascripts/modules/sticky-element-container.spec.js +++ b/spec/javascripts/modules/sticky-element-container.spec.js @@ -21,7 +21,7 @@ describe('A sticky-element-container module', function () { describe('on desktop', function () { beforeEach(function () { - instance._getWindowDimensions = function () { + instance.getWindowDimensions = function () { return { height: 768, width: 1024 @@ -36,7 +36,7 @@ describe('A sticky-element-container module', function () { }) it('shows the element, stuck to the window, when scrolled in the middle', function () { - instance._getWindowPositions = function () { + instance.getWindowPositions = function () { return { scrollTop: 5000 } @@ -48,7 +48,7 @@ describe('A sticky-element-container module', function () { }) it('shows the element, stuck to the parent, when scrolled at the bottom', function () { - instance._getWindowPositions = function () { + instance.getWindowPositions = function () { return { scrollTop: 9800 } From d662dd48edc7c008b98b20dab1dfc1ea68b35cc4 Mon Sep 17 00:00:00 2001 From: Graham Lewis <44037625+gclssvglx@users.noreply.github.com> Date: Mon, 1 Nov 2021 09:52:31 +0000 Subject: [PATCH 3/4] Add bug fix for incorrect height being reported The reported vanilla replacement for .height() is el.offsetHeight - and this works correctly in the production code. However, in the tests this gives us an incorrect height (0) as the height of the element is set via a style attribute here. This change adds a fix for the element height calculation that works in both the tests and production code. --- app/assets/javascripts/modules/sticky-element-container.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/modules/sticky-element-container.js b/app/assets/javascripts/modules/sticky-element-container.js index cb15df6e4..7347e4713 100644 --- a/app/assets/javascripts/modules/sticky-element-container.js +++ b/app/assets/javascripts/modules/sticky-element-container.js @@ -62,8 +62,9 @@ hasScrolled = true var windowDimensions = self.getWindowDimensions() + var elementHeight = el.offsetHeight || parseFloat(el.style.height.replace('px', '')) startPosition = el.offsetTop - stopPosition = el.offsetTop + el.offsetHeight - windowDimensions.height + stopPosition = el.offsetTop + elementHeight - windowDimensions.height } } From f699ef891d1d9d9b4dc2e1ef76e85299f75afb3c Mon Sep 17 00:00:00 2001 From: Graham Lewis <44037625+gclssvglx@users.noreply.github.com> Date: Wed, 3 Nov 2021 10:42:06 +0000 Subject: [PATCH 4/4] Rename variables to clarify their intent This change renames: 'el' to 'wrapper', and 'element' to 'stickyElement'. 'root' is removed as we can now reference the global 'window' object instead. Clarify the intent and concept of the variables - remove an unrequired variable. --- .../modules/sticky-element-container.js | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/app/assets/javascripts/modules/sticky-element-container.js b/app/assets/javascripts/modules/sticky-element-container.js index 7347e4713..0357b70ef 100644 --- a/app/assets/javascripts/modules/sticky-element-container.js +++ b/app/assets/javascripts/modules/sticky-element-container.js @@ -7,7 +7,7 @@ Use 'data-module="sticky-element-container"' to instantiate, and add `[data-sticky-element]` to the child you want to position. */ -(function (Modules, root) { +(function (Modules) { 'use strict' Modules.StickyElementContainer = function () { @@ -15,20 +15,20 @@ self.getWindowDimensions = function () { return { - height: root.innerHeight, - width: root.innerWidth + height: window.innerHeight, + width: window.innerWidth } } self.getWindowPositions = function () { return { - scrollTop: root.scrollY + scrollTop: window.scrollY } } self.start = function ($el) { - var el = $el[0] - var element = el.querySelector('[data-sticky-element]') + var wrapper = $el[0] + var stickyElement = wrapper.querySelector('[data-sticky-element]') var hasResized = true var hasScrolled = true @@ -39,13 +39,13 @@ initialise() function initialise () { - root.onresize = onResize - root.onscroll = onScroll + window.onresize = onResize + window.onscroll = onScroll setInterval(checkResize, interval) setInterval(checkScroll, interval) checkResize() checkScroll() - element.classList.add('sticky-element--enabled') + stickyElement.classList.add('sticky-element--enabled') } function onResize () { @@ -62,9 +62,9 @@ hasScrolled = true var windowDimensions = self.getWindowDimensions() - var elementHeight = el.offsetHeight || parseFloat(el.style.height.replace('px', '')) - startPosition = el.offsetTop - stopPosition = el.offsetTop + elementHeight - windowDimensions.height + var elementHeight = wrapper.offsetHeight || parseFloat(wrapper.style.height.replace('px', '')) + startPosition = wrapper.offsetTop + stopPosition = wrapper.offsetTop + elementHeight - windowDimensions.height } } @@ -98,20 +98,20 @@ } function stickToWindow () { - element.classList.add('sticky-element--stuck-to-window') + stickyElement.classList.add('sticky-element--stuck-to-window') } function stickToParent () { - element.classList.remove('sticky-element--stuck-to-window') + stickyElement.classList.remove('sticky-element--stuck-to-window') } function show () { - element.classList.remove('sticky-element--hidden') + stickyElement.classList.remove('sticky-element--hidden') } function hide () { - element.classList.add('sticky-element--hidden') + stickyElement.classList.add('sticky-element--hidden') } } } -})(window.GOVUK.Modules, window) +})(window.GOVUK.Modules)