Skip to content

Conversation

@muffinista
Copy link
Contributor

@muffinista muffinista commented Oct 19, 2024

Addresses processing/p5.js-website#527

Changes:

The link to the page about creating accessible labels is broken and this seems like the proper one.

Screenshots of the change:

PR Checklist

@welcome
Copy link

welcome bot commented Oct 19, 2024

🎉 Thanks for opening this pull request! Please check out our contributing guidelines if you haven't already. And be sure to add yourself to the list of contributors on the readme page!

Copy link
Collaborator

@perminder-17 perminder-17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for taking on this task. When you run grunt yui:dev, it should start the reference website with the updated code and examples. However, I'm still encountering a broken link when I try to run it. Could you please verify the correct path?

Additionally, I'm unsure if we have the tutorial page included in the p5.js repository. If it’s missing, we might need to use absolute URLs, such as:

<a href="https://p5js.org/tutorials/writing-accessible-canvas-descriptions/">Writing accessible canvas descriptions</a>

@davepagurek
Copy link
Contributor

davepagurek commented Oct 20, 2024

I think the relative path from / technically will work once it gets merged and then parsed in the website repo, but I think as @perminder-17 suggests, an absolute path probably is the way to go here so that it's easier to test.

The old link is broken and this seems like the proper one. Addresses p5.js-website#527
@muffinista muffinista force-pushed the point-to-accessibility-tutorial branch from fc137e6 to 30f10f1 Compare October 20, 2024 21:47
@muffinista
Copy link
Contributor Author

Thanks very much for the feedback! I've switched this to an absolute link, and I properly built/tested locally, and I believe everything is working as expected.

@perminder-17 perminder-17 self-requested a review November 7, 2024 02:14
Copy link
Collaborator

@perminder-17 perminder-17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am really sorry for the delay in the review. LGTM, thanks for your work and your patience.

@davepagurek davepagurek merged commit 5526350 into processing:main Nov 8, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants