feat: add accessibility nightmare beginner adventure#72
Conversation
Signed-off-by: Ehtasham-Yasin <ehtasham.yasin.dev@gmail.com>
|
Thanks a lot for reopening! 😊 As said, I'll do a review tomorrow. |
KatharinaSick
left a comment
KatharinaSick
left a comment
There was a problem hiding this comment.
Thanks a lot for contributing this adventure! It's honestly really well made. I just gave it a try and had a lot of fun playing. And even better: I learned something new 😊
I left a couple of comments but only one of them is really big I think. I'm not a frontend dev so if any of the comments doesn't make any sense, feel free to ignore it. Just add a little comment explaining what's going on in that case please.
| - id: explore | ||
| title: "Explore the Broken Storefront" | ||
| content: | | ||
| Open ShopSmart on port 5173 and navigate the page using only the keyboard. |
There was a problem hiding this comment.
I just started the devcontainer and there's no service running on port 5173 (post-create & -start succeeded).
If that's intentional, please add some instructions on how to start the service please.
(I just tried with npm run dev and it worked)
There was a problem hiding this comment.
Thanks for catching this! You’re right, it wasn’t behaving as intended. I updated the post-start.sh script to wait until the Vite dev server is actually available before continuing, and it now reports a clear error if startup fails. I re-tested it in a fresh devcontainer, and the app now starts automatically on port 5173. Thanks for pointing this out!
There was a problem hiding this comment.
I just started an other GitHub Codespace and there's nothing running on port 5173. Maybe it would be better to just add a Makefile target to start the app and add a "start app" section to the beginning of how_to_play in the docs?
|
|
||
| ```bash | ||
| npm run test:a11y | ||
| npm run test:lighthouse |
There was a problem hiding this comment.
Running this one fails for me because it needs a Chrome installation. Is that intentional?
@KatharinaSick ➜ .../adventures/planned/adventure-accessibility-nightmare/beginner (feature/accessibility-nightmare-reopen) $ npm run test:lighthouse
> accessibility-nightmare-beginner@1.0.0 test:lighthouse
> lhci autorun
✅ .lighthouseci/ directory writable
✅ Configuration file found
❌ Chrome installation not found
⚠️ GitHub token not set
Healthcheck failed!
There was a problem hiding this comment.
Chrome being unavailable was not intentional. I updated the devcontainer setup so LHCI can find and launch the Playwright-installed Chromium with container-safe flags. I verified that Lighthouse now completes the audit successfully. The remaining exit code 1 is expected for the starter challenge because the intentionally inaccessible page currently scores 0.80, below the required 0.95 threshold; learners are meant to improve that score during the challenge.
There was a problem hiding this comment.
I just tried in a fresh GH Codespace but it still shows that no chrome installation is found. I then checked your changes and found that the env var CHROME_PATH should be set but for me it actually shows up empty.
I then executed the command that's used to set it and it returns nothing:
@KatharinaSick ➜ .../adventures/planned/adventure-accessibility-nightmare/beginner (feature/accessibility-nightmare-reopen) $ find "$HOME/.cache/ms-playwright" -type f -path '*/chrome-linux/chrome' | head -1
@KatharinaSick ➜ .../adventures/planned/adventure-accessibility-nightmare/beginner (feature/accessibility-nightmare-reopen) $ 
|
Thankyou so much for the kind feedback and for taking the time to try the adventure.I went through all of your comments and addressed them, including the Chrome/Lighthouse setup, the accessibility test coverage for visible focus indicators, and keeping the verification objectives in sync with the documentation. Your feedback was very helpful. Thanks again for the detailed review! |
|
Thank you for the quick updates! Could it be that you didn't push your most recent changes? I can't see them. |
Thanks for feedback. I have now added all requested changes and pushed them to correct branch. I also tested Playwright and Lighthouse and both are working correctly with the starter state. |
|
Thank you! :) |
Thanks, I fixed the two remaining issues. I added a Makefile target and start instructions for ShopSmart, and changed the Chrome path detection to use Playwright's actual Chromium executable path instead of assuming a fixed cache location. I tested both changes locally and pushed them to the PR branch. |
KatharinaSick
left a comment
KatharinaSick
left a comment
There was a problem hiding this comment.
I just played through it again and it's fun to play! Thanks for the quick adjustments :)
Just a minor fix to the Makefile and we're good to merge. Please don't forget to sign your commits.
| @@ -1,8 +1,8 @@ | |||
| # ============================================================================== | |||
| # Open Ecosystem Challenges | |||
There was a problem hiding this comment.
Please add a dedicated Makefile inside the beginner directory so that it can be used in the challenge. Also, I'd suggest to just call the target "app" or something similar so that the same target can be used in the next levels too but that's up to you :)
You can check dead-reckoning or lex-imperfecta for Makefile examples
There was a problem hiding this comment.
It would be great if you could add a verify target too so that the experience is the same as in previous adventures.
Thanks. I've added a dedicated Makefile for the beginner adventure with app and verify targets, and updated the root Makefile to delegate to the local app target. The changes have been pushed with a signed commit |
|
Amazing! Thank you! 😊🎉 |
2 participants
Reopens the implementation for #65 after PR #70 was accidentally merged and reverted in #71.
Closes #65