Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

starred courses popover in navbar throws js error on every page. #759

Closed
davidscotson opened this issue Nov 18, 2024 · 2 comments
Closed
Assignees
Labels
bug Something which isn't working

Comments

@davidscotson
Copy link

Describe the bug
On our 4.5 test site, enabling the "Show starred courses popover in the navbar" option in the settings causes a JS console error on every page.

To Reproduce
Steps to reproduce the behavior:

  1. Go to /admin/settings.php?section=theme_boost_union_feel
  2. Scroll down to Navbar section.
  3. Enable the 'Show starred courses popover in the navbar (shownavbarstarredcourses)' option and save
  4. On the next page load there will a js console error, Uncaught TypeError: controller.registerEventListeners is not a function

Additional context
When investigating this I switched between themes using the ?theme= functionality. The error persisted across other themes, and the star continued to be displayed in the navbar on Classic and vanilla Boost themes. After selecting a different theme in the settings to switch the theme entirely, the star in the navbar was still visible and active if enabled in Boost Union. I'm assuming that's not intended either.

@davidscotson davidscotson added the new Something which has been reported but has not yet beeen triaged by the team label Nov 18, 2024
@abias abias self-assigned this Nov 19, 2024
@abias abias added bug Something which isn't working and removed new Something which has been reported but has not yet beeen triaged by the team labels Nov 19, 2024
@abias abias moved this to To do REQ in Boost Union Planning Board Nov 19, 2024
@abias abias moved this from To do REQ to Ready for REVIEW in Boost Union Planning Board Nov 19, 2024
abias added a commit that referenced this issue Nov 19, 2024
…if Boost Union or Boost Union child is active, resolves #759.
abias added a commit that referenced this issue Nov 19, 2024
…if Boost Union or Boost Union child is active, resolves #759.
@abias abias moved this from Ready for REVIEW to In Progress REVIEW in Boost Union Planning Board Nov 19, 2024
@abias
Copy link
Member

abias commented Nov 19, 2024

Hi @davidscotson ,

many thanks for raising this issue.

I can reproduce the JavaScript console error on Moodle 4.5 but also on older versions down to 4.1. I will investitate and hopefully fix it shortly.

Regarding your additional comment that the starred courses feature is shown for other active themes as well, you are completely right. We have missed that detail in the initial implementation.
I have just pushed a PR on #762 to fix that and have created #761 for some volunteer to cover it with tests.

Cheers,
Alex

abias added a commit that referenced this issue Nov 19, 2024
@abias
Copy link
Member

abias commented Nov 19, 2024

I think I found the reason for the JS console error as well:
When building the starred courses feature, a JS line had been adopted by mistake from the notifications popover:
d461823

This is removed now in the updated PR.

abias added a commit that referenced this issue Nov 19, 2024
…if Boost Union or Boost Union child is active, resolves #759.
abias added a commit that referenced this issue Nov 19, 2024
@abias abias closed this as completed in aae86d1 Nov 20, 2024
@github-project-automation github-project-automation bot moved this from In Progress REVIEW to Ready for BACKPORT in Boost Union Planning Board Nov 20, 2024
abias added a commit that referenced this issue Nov 20, 2024
…if Boost Union or Boost Union child is active, resolves #759. (#762)

* Bugfix: The starred courses popover in the navbar must only be shown if Boost Union or Boost Union child is active, resolves #759.

* Bugfix: The starred courses popover showed a JavaScript error in the browser JS console, resolves #759
abias added a commit that referenced this issue Nov 20, 2024
…if Boost Union or Boost Union child is active, resolves #759. (#762)

* Bugfix: The starred courses popover in the navbar must only be shown if Boost Union or Boost Union child is active, resolves #759.

* Bugfix: The starred courses popover showed a JavaScript error in the browser JS console, resolves #759
abias added a commit that referenced this issue Nov 20, 2024
…if Boost Union or Boost Union child is active, resolves #759. (#762)

* Bugfix: The starred courses popover in the navbar must only be shown if Boost Union or Boost Union child is active, resolves #759.

* Bugfix: The starred courses popover showed a JavaScript error in the browser JS console, resolves #759
abias added a commit that referenced this issue Nov 20, 2024
…if Boost Union or Boost Union child is active, resolves #759. (#762)

* Bugfix: The starred courses popover in the navbar must only be shown if Boost Union or Boost Union child is active, resolves #759.

* Bugfix: The starred courses popover showed a JavaScript error in the browser JS console, resolves #759
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something which isn't working
Projects
Status: CLOSED
Development

No branches or pull requests

2 participants