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

Scrollspy is not working with TheAgencyTheme #12688

Closed
DemeSzabolcs opened this issue Oct 22, 2022 · 2 comments · Fixed by #12758
Closed

Scrollspy is not working with TheAgencyTheme #12688

DemeSzabolcs opened this issue Oct 22, 2022 · 2 comments · Fixed by #12758
Labels
Milestone

Comments

@DemeSzabolcs
Copy link
Contributor

DemeSzabolcs commented Oct 22, 2022

Describe the bug

Scrollspy is not working with TheAgencyTheme

Srcollspy should add the active class to the link menu items when reaching that anchor while scrolling. Scrollspy can only handle anchor links with # beginning. Currently, the anchor links are in ~/#anchor format.

I didn't open an issue in the theme's repository, because there, the links are static (matching the #anchor format). This is only problem in Orchard Core because of the unique implementation of the menu.

I didn't open an issue in the bootstrap repository, because there were multiple closed PRs about this issue.
They suggested using the data-bs-target attribute on the <a> tags. So in this case we would need to add the data-bs-target = #anchor attribute to every anchor link.

This would work because the Scrollspy selector is this:

const queries = SELECTOR_LINK_ITEMS.split(',')
      .map(selector => `${selector}[data-bs-target="${target}"],${selector}[href="${target}"]`)

If we could add data-bs-targets dynamically in liquid (or from the recipe) with the appropriate values, it would even work on tenants.

To Reproduce

Steps to reproduce the behavior:

  1. Set the theme to the Agency Theme
  2. Open your browser's developer tools.
  3. Scroll down.
  4. See that the menu items are not changing (one of them that corresponds to the current section should be active).
  5. See that there are errors inside scrollspy.js. (line 224 where link is null, because it can't find <a> tags matching the href pattern that begins with #).

Expected behavior

Scrollspy should work correctly with the menu items, changing them to active after scrolling to the given section.

Screenshots

Menu item not being activated:
image

Scrollspy error:
image
image

Orchard Core link menu items in The Agency theme:
image

@sebastienros
Copy link
Member

Can we check if the issue was fixed in the template source repository?
Or if it's just liquid to adapt then can you create a PR for it?

@DemeSzabolcs
Copy link
Contributor Author

Can we check if the issue was fixed in the template source repository?

In the original template source repository, these links are static and hard coded, they are in a good format (so they don't have /). So this problem is only present here in Orchard Core because they are LinkMenuItems. I tried to remove the / from them inside the recipe, but they need to have it otherwise it will throw an error because of an invalid Url.

However, I fixed the problem by adding the data-bs-target attribute with the appropriate value inside the MenuItemLink-LinkMenuItem.liquid if the menu item link is an anchor link.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants