Use the dialog element or the aria-modal attribute for the dialog modal of the tour tips

[rpkoller]

From the perspective of screenreader users the dialog modal tour tips in shephard have one problem. At the moment they are wrapped in a regular div element. The elements in the background of the highlighted component and the tour tip dialog modal when a tour is started are not clickable. Problem for screenreader users, the elements in the background are still accessible and are not excluded from the accessibility object model. for the screenreader user it is impossible to distinguish between what belongs to a dialog modal and what are elements in the background of the tour tip:

sheperd.mp4

A way to tackle that would be making the switch from the div element to the dialog element for the dialog modals of the tour tips. the support is solid meanwhile and it is supported in all major browsers: https://caniuse.com/?search=dialog . there is also a highly recommended presentation outlining the ins and outs of the dialog element and the popover attribute.https://talks.hiddedevries.nl/VZNuWJ/dialog-dilemmas-and-modal-mischief-on-building-popovers#skMZ7WI

[mgifford]

Always good to just use semantic HTML rather than a meaningless div.

[chuckcarpenter]

@rpkoller I don't disagree and we love semantic HTML, would you be open to submitting a PR to kick things off?

[rpkoller]

yeah even though i not identify as a developer, i am more into content strategy and ux/a11y, but i would take a look and give the PR a try. the only detail i am struggling with right now is setting up an environment where i am able to see the changes in action. i already pulled the repo and spun it up in ddev. the only thing i would need confirmation for is if that is the page that is served on port 10001?

cuz within the landing folder there are also astro pages for for example the demo page shown in my linked video (if that page is served would it use the build shepherd source as well so i would be able to assess my changes upfront?). setting up the environment to get things runnings is most often the most challenging part for me therefore sorry for the probably basic question.

[chuckcarpenter]

@rpkoller that site is really just the placeholder for cypress integration testing. You want to first run a build at the root pnpm build and then run dev in the landing directory (it will use that built package). I agree it's not straightforward for new folks.

[rpkoller]

thanks @chuckcarpenter, except the run dev i already did all that. i was following along https://github.com/shepherd-pro/shepherd/blob/main/CONTRIBUTING.md . steps i've followed:

• ran git clone [email protected]:shepherd-pro/shepherd.git . within an empty directory.
• created config for shepherd in the ddev project (setting the node version and the port)

# config.shepard.yaml
nodejs_version: "18"
web_extra_exposed_ports:
  - name: shepard
    container_port: 10001
    http_port: 10001
    https_port: 10001

• enabled pnpm in ddev with ddev . corepack enable pnpm
• ddev . pnpm i
• ddev . pnpm build

Initially i tried running ddev .pnpm startin the root folder which lead me to the screenshot from my previous comment. I then followed your suggestion and did cd landing && ddev . pnpm run dev but i end up again on the same page shown in the placeholder screenshot from before. also tried http://shepherd.ddev.site:10001/landingbut that lead to

404 Not Found

/var/www/html/test/cypress/dummy/landing

(rollup-plugin-serve)

my two questions would be based how i understand things at this point. within landing directory it looks like there are all the assets building the same page you find at https://blog.shepherdpro.com/demo/ locally? and if it would be working to access that blog demo page (if that is the right assumption on my end) would shepherd that is used on the demo page be using a compiled version of ./shepherd.js so all the changes to the shepherd source are being reflected in the demo?

p.s. i am also happy to update the docs for onboarding after i figured things out to make it potentially more clear to others as well. as you'Ve already mentioned that things are not straightforward for new folks.

[chuckcarpenter]

@rpkoller apologies, but I'm not familiar with ddev. So, I don't know if that gets you a local domain like that, but running the landing site in Astro should give you the site running at http://localhost:4321

[rpkoller]

no worries. yep ddev is providing a local domain. it is basically a docker based local development environment. i dont have node installed on my computer i only use it within ddevs web container for example. (and all those steps setting up the environment for shepherd contributions could then be set up and automated in a ddev addon)

i got things working. or at least i realized where my main problem was. for some reason i havent figured out running the instructions from outside the container resulted in differing results than running them inside the container. i've only executed the command from outside the container at first. now from within the container things look better and more clear now and i run into a meaningful error.

after running pnpm build in the root directory i've checked for the dist directory. it was created within shepherd.jswith three sub directories:

./shepherd.js/dist/cjs
./shepherd.js/dist/css
./shepherd.js/dist/esm

but when i run pnpm run dev inside the landing directory i now run into:

✘ [ERROR] Missing "./dist/shepherd.mjs" specifier in "shepherd.js" package [plugin vite:dep-scan]

    html:/var/www/html/landing/src/pages/demo.astro:1:7:
      1 │ import "shepherd.js/dist/shepherd.mjs"
        ╵        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

  This error came from the "onResolve" callback registered here:

    ../node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1150:20:
      1150 │       let promise = setup({
           ╵                     ^

    at setup (file:///var/www/html/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:49466:13)
    at handlePlugins (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1150:21)
    at buildOrContextImpl (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:873:5)
    at Object.buildOrContext (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:699:5)
    at /var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:2032:68
    at new Promise (<anonymous>)
    at Object.context (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:2032:27)
    at Object.context (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1874:58)
    at prepareEsbuildScanner (file:///var/www/html/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/node/chunks/dep-mCdpKltl.js:49263:24)


    at failureErrorWithLog (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1472:15)
    at /var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:945:25
    at runOnEndCallbacks (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:1315:45)
    at buildResponseToResult (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:943:7)
    at /var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:955:9
    at new Promise (<anonymous>)
    at requestCallbacks.on-end (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:954:54)
    at handleRequest (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:647:17)
    at handleIncomingPacket (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:672:7)
    at Socket.readFromStdout (/var/www/html/node_modules/.pnpm/[email protected]/node_modules/esbuild/lib/main.js:600:7)

 update  ▶ New version of Astro available: 4.15.3
  Run pnpm dlx @astrojs/upgrade to update

looks like the import statement is pointing to a none existent directory?

import "shepherd.js/dist/shepherd.mjs"

but i've tried and changed the directory to

import "shepherd.js/dist/esm/shepherd.mjs"

but that lead to the same error which is kind of unexpected.

p.s. i am using the nodejs version 18.19.0 based on the .tools-versions file.