Skip to content

Commit

Permalink
Upgrade: Fix the scrollspy which broke on Moodle 4.3, solves #420.
Browse files Browse the repository at this point in the history
  • Loading branch information
abias committed Oct 31, 2023
1 parent ec79542 commit 7da8096
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 8 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ Changes

### Unreleased

* 2023-10-28 - Upgrade: Fix the scrollspy which broke on Moodle 4.3, solves #420.
* 2023-10-28 - Upgrade: Fix a broken Behat test with the additional resources setting.
* 2023-10-28 - Upgrade: Fix the back-to-top button which broke on Moodle 4.3, solves #419.
* 2023-10-27 - Upgrade: Fix a broken Behat test with modal confirm dialogues.
Expand Down
2 changes: 1 addition & 1 deletion amd/build/scrollspy.min.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion amd/build/scrollspy.min.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions amd/src/scrollspy.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const initScrollSpy = () => {

window.sessionStorage.setItem('edittoggled', true);

let viewporttop = document.getElementById('page').scrollTop;
let viewporttop = window.scrollY;
let closest = null;
let closestoffset = null;

Expand Down Expand Up @@ -76,7 +76,7 @@ const initScrollSpy = () => {
let closest = document.getElementById(closestid);
let y = closest.offsetTop + parseInt(closestdelta);

document.getElementById('page').scrollTo(0, y);
window.scrollTo(0, y);
}

window.sessionStorage.removeItem('edittoggled');
Expand Down
39 changes: 35 additions & 4 deletions tests/behat/behat_theme_boost_union_base_general.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public function i_scroll_page_to_dom_element_with_id($selector) {
$scrolljs = "(function(){
let element = document.getElementById('$selector');
let y = element.offsetTop;
document.getElementById('page').scrollTo(0, y);
window.scrollTo(0, y);
})();";
try {
$this->getSession()->executeScript($scrolljs);
Expand All @@ -97,6 +97,37 @@ public function i_scroll_page_to_dom_element_with_id($selector) {
}
}

/**
* Make the navbar fixed.
*
* This is not really a real Behat step, but it is necessary for the test of the scrollspy feature in the
* 'Setting: Scrollspy - Enable "Scrollspy"' scenario.
*
* The problem is there:
* 1. The 'I scroll page to DOM element with ID' step scrolls the page down as it is needed for the scenario.
* 2. The 'I turn editing mode on' scrolls the page up again to click the edit button as the navbar is not fixed
* in Behat runs.
*
* Because of this, the scrollspy would save the wrong viewport-y value and the scenario would fail.
* Against this background, we have to make the navbar fixed just before turning editing on.
* And that's what this step is here for.
*
* @copyright 2023 Alexander Bias <[email protected]>
* @Then I make the navbar fixed
* @return void
* @throws Exception
*/
public function i_make_the_navbar_fixed() {
$fixedjs = "(function(){
document.querySelector('nav.navbar').style.position = 'fixed';
})();";
try {
$this->getSession()->executeScript($fixedjs);
} catch (Exception $e) {
throw new \Exception('Making the navbar fixed failed');
}
}

/**
* Checks if the top of the page is at the top of the viewport.
*
Expand All @@ -107,7 +138,7 @@ public function i_scroll_page_to_dom_element_with_id($selector) {
public function page_top_is_at_top_of_viewport() {
$posviewportjs = "
return (
document.getElementById('page').scrollTop
window.scrollY
)
";
$positionviewport = $this->evaluate_script($posviewportjs);
Expand All @@ -126,7 +157,7 @@ public function page_top_is_at_top_of_viewport() {
public function page_top_is_not_at_top_of_viewport() {
$posviewportjs = "
return (
document.getElementById('page').scrollTop
window.scrollY
)
";
$positionviewport = $this->evaluate_script($posviewportjs);
Expand All @@ -152,7 +183,7 @@ public function dom_element_is_at_top_of_viewport($selector) {
$positionelement = $this->evaluate_script($poselementjs);
$posviewportjs = "
return (
document.getElementById('page').scrollTop
window.scrollY
)
";
$positionviewport = $this->evaluate_script($posviewportjs);
Expand Down
2 changes: 2 additions & 0 deletions tests/behat/theme_boost_union_feelsettings_navigation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,12 @@ Feature: Configuring the theme_boost_union plugin for the "Navigation" tab on th
And I press "Purge all caches"
And I am on "Course 1" course homepage
And I scroll page to DOM element with ID "section-4"
And I make the navbar fixed
And I turn editing mode on
And I wait "2" seconds
Then DOM element "section-4" is at the top of the viewport
And page top is not at the top of the viewport
And I make the navbar fixed
And I turn editing mode off
And I wait "2" seconds
Then DOM element "section-4" is at the top of the viewport
Expand Down

0 comments on commit 7da8096

Please sign in to comment.