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

Docs: Example "Two-level menu with toggle" not working #6

Closed
mandrasch opened this issue Feb 3, 2022 · 9 comments
Closed

Docs: Example "Two-level menu with toggle" not working #6

mandrasch opened this issue Feb 3, 2022 · 9 comments
Labels
documentation Improvements or additions to documentation

Comments

@mandrasch
Copy link

Thanks very much for providing this as open source! 👍

Summary

I tried to setup the example Two-level menu with toggle in a Codepen, but I get

TypeError: Cannot read properties of null (reading 'addEventListener')
    at _baseMenu.js:881:25

Error results from .dom.link:

 _handleFocus() {
    this.elements.menuItems.forEach((menuItem, index) => {
      menuItem.dom.link.addEventListener("focus", () => {

I already changed this line from docs

const menu = new AccessibleMenu.Bootstrap5DisclosureMenu({

to

const menu = new AccessibleMenuBootstrap5.Bootstrap5DisclosureMenu({

initialize it correctly.

I load the bootstrap css and the following js in codepen:

https://cdnjs.cloudflare.com/ajax/libs/bootstrap/5.1.3/js/bootstrap.min.js
https://cdn.jsdelivr.net/npm/accessible-menu-bootstrap-5@1.0.0/dist/accessible-menu-bs5.min.js

Live: Codepen Live
Editor: Codepen Editor

Is there something I missed? I'm not familiar with dom.link of elements?

Thanks in advance!

@mandrasch mandrasch added the documentation Improvements or additions to documentation label Feb 3, 2022
@mandrasch mandrasch changed the title Docs: [Brief Description] Docs: Example "Two-level menu with toggle" not working Feb 3, 2022
@NickDJM
Copy link
Owner

NickDJM commented Feb 7, 2022

This sounds like a case of me copy pasting examples from the other projects and not actually checking to make sure they work.

I'll double check all the examples to make sure they are accurate.

@NickDJM
Copy link
Owner

NickDJM commented Feb 14, 2022

@mandrasch I have a PR open with example fixes. I've tested them and it looks good. If you have time do you want to give them a try as well? I'm hoping this will resolve the issue (it was 100% the case that I just copy pasted the examples from the base project and never updated them for Bootstrap).

This also was the case for the BS4 one as well, so we have 2 fixes coming out!

@mandrasch
Copy link
Author

Thanks very much! 👍 I'll have a look (today or within next few days)

@mandrasch
Copy link
Author

Just a quick notice: I created a repo for the examples of bs4, took the code from branch fix-examples

Repo: https://github.com/mandrasch/accessible-menu-bootstrap-examples
Live: https://mandrasch.github.io/accessible-menu-bootstrap-examples/

Implementation is (inline JS) https://github.com/mandrasch/accessible-menu-bootstrap-examples/blob/main/bs4-examples.njk

Had to change some small things to get it working. I'll will post these here later. :-)

@mandrasch
Copy link
Author

mandrasch commented Feb 18, 2022

Things I had to change (bootstrap 4):

  1. For https://github.com/NickDJM/accessible-menu-bootstrap-4/blob/fix-examples/docs/basics/single-level-menu.md, I changed new command and query selector in JS:
const menu = new AccessibleMenuBootstrap4.Bootstrap4DisclosureMenu({

Changed the selector, it was set to menu:

  menuElement: document.querySelector("#main-nav .navbar-nav"),
  1. For https://github.com/NickDJM/accessible-menu-bootstrap-4/blob/fix-examples/docs/basics/single-level-menu-with-toggle.md I changed the init as well:
const menu = new AccessibleMenuBootstrap4.Bootstrap4DisclosureMenu({
  1. For https://github.com/NickDJM/accessible-menu-bootstrap-4/blob/fix-examples/docs/basics/two-level-menu-with-toggle.md changed this as well:
const menu = new AccessibleMenuBootstrap4.Bootstrap4Menubar({
  1. Added navbar-light and bg-light class to main-navs in all examples for bs styling:
<nav id="main-nav-single-level" aria-label="Main" class="navbar navbar-light bg-light">
  1. Noticed that there is the dropdown icon in text and the dropdown-toggle css (:after) from bs
    -_Bootstrap_4_examples

@NickDJM
Copy link
Owner

NickDJM commented Mar 8, 2022

Haven't had a chance to look at this yet- been super busy. I should have some free time tonight though.

NickDJM added a commit to NickDJM/accessible-menu-bootstrap-4 that referenced this issue Mar 10, 2022
@NickDJM
Copy link
Owner

NickDJM commented Mar 10, 2022

@mandrasch Updated the fix-examples branch with the all changes above (excluding 4 since that's just colour changes).

Are you planning on keeping hose examples up? If so, we can link to them in the docs.

@mandrasch
Copy link
Author

@NickDJM Yes sure, I can keep this repo up. :)

Just removed dropdown html icons as well.

@NickDJM
Copy link
Owner

NickDJM commented Mar 22, 2022

Examples have been updated and merged.

@NickDJM NickDJM closed this as completed Mar 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

2 participants