-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix screen reader features #2694
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
Conversation
@raclim @Qianqianye @davepagurek @lindapaiste what do y'all think about this approach? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking this on! I think this looks good, I just have one question (maybe for @lindapaiste?) about script tag order to try to handle as many cases as possible.
}; | ||
p5.prototype.registerMethod('afterSetup', p5.prototype.ensureAccessibleCanvas);`; | ||
scriptElement.innerHTML = fxn; | ||
sketchDoc.head.appendChild(scriptElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think there's any harm in putting this at the end of the body in case people rearrange the imports of their sketch and import p5 in the body too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it, I think this is good to go then!
@lindapaiste @davepagurek just checking in. Can I help to improve this pull request? Also curious to hear your thoughts on #2702 which would be affected. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for working on this and for everyone's input here! Since it's already received a review, I'm going to merge this in!
Fixes #2188
Changes:
I modified the
injectLocalFiles()
function inclient/Preview/EmbedFrame.jsx
to usetextOutput()
andgridOutput()
.I have verified that this pull request:
npm run lint
)npm run test
)develop
branch.Fixes #123