-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Accessibility - Transition from interceptor to accessibility library #508
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
Changes from 10 commits
344e50e
cff4819
dcda574
cf76989
1ae2c6f
7097808
d98e700
e510b8f
a2f70a4
c33d68c
bb846c2
4e8f9e4
2e97dd2
356921b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -198,49 +198,31 @@ class PreviewFrame extends React.Component { | |
this.resolveScripts(sketchDoc, resolvedFiles); | ||
this.resolveStyles(sketchDoc, resolvedFiles); | ||
|
||
let scriptsToInject = [ | ||
const scriptsToInject = [ | ||
'/loop-protect.min.js', | ||
'/hijackConsole.js' | ||
]; | ||
if ( | ||
this.props.isAccessibleOutputPlaying || | ||
((this.props.textOutput || this.props.gridOutput || this.props.soundOutput) && this.props.isPlaying)) { | ||
let interceptorScripts = []; | ||
interceptorScripts = [ | ||
'/p5-interceptor/registry.js', | ||
'/p5-interceptor/loadData.js', | ||
'/p5-interceptor/interceptorHelperFunctions.js', | ||
'/p5-interceptor/baseInterceptor.js', | ||
'/p5-interceptor/entities/entity.min.js', | ||
'/p5-interceptor/ntc.min.js' | ||
]; | ||
if (!this.props.textOutput && !this.props.gridOutput && !this.props.soundOutput) { | ||
this.props.setTextOutput(true); | ||
} | ||
if (this.props.textOutput) { | ||
let textInterceptorScripts = []; | ||
textInterceptorScripts = [ | ||
'/p5-interceptor/textInterceptor/interceptorFunctions.js', | ||
'/p5-interceptor/textInterceptor/interceptorP5.js' | ||
]; | ||
interceptorScripts = interceptorScripts.concat(textInterceptorScripts); | ||
} | ||
if (this.props.gridOutput) { | ||
let gridInterceptorScripts = []; | ||
gridInterceptorScripts = [ | ||
'/p5-interceptor/gridInterceptor/interceptorFunctions.js', | ||
'/p5-interceptor/gridInterceptor/interceptorP5.js' | ||
]; | ||
interceptorScripts = interceptorScripts.concat(gridInterceptorScripts); | ||
} | ||
if (this.props.soundOutput) { | ||
let soundInterceptorScripts = []; | ||
soundInterceptorScripts = [ | ||
'/p5-interceptor/soundInterceptor/interceptorP5.js' | ||
]; | ||
interceptorScripts = interceptorScripts.concat(soundInterceptorScripts); | ||
} | ||
scriptsToInject = scriptsToInject.concat(interceptorScripts); | ||
const accessiblelib = sketchDoc.createElement('script'); | ||
accessiblelib.setAttribute('src', 'https://cdn.rawgit.com/MathuraMG/p5-accessibility/e856365c/dist/p5-accessibility.js'); | ||
if (this.props.textOutput) { | ||
sketchDoc.getElementById('accessibility-library').appendChild(accessiblelib); | ||
const textSection = sketchDoc.createElement('section'); | ||
textSection.setAttribute('id', 'textOutput-content'); | ||
sketchDoc.getElementById('accessible-outputs').appendChild(textSection); | ||
this.iframeElement.focus(); | ||
} | ||
if (this.props.gridOutput) { | ||
sketchDoc.getElementById('accessibility-library').appendChild(accessiblelib); | ||
const gridSection = sketchDoc.createElement('section'); | ||
gridSection.setAttribute('id', 'gridOutput-content'); | ||
sketchDoc.getElementById('accessible-outputs').appendChild(gridSection); | ||
this.iframeElement.focus(); | ||
} | ||
if (this.props.soundOutput) { | ||
sketchDoc.getElementById('accessibility-library').appendChild(accessiblelib); | ||
const soundSection = sketchDoc.createElement('section'); | ||
soundSection.setAttribute('id', 'soundOutput-content'); | ||
sketchDoc.getElementById('accessible-outputs').appendChild(soundSection); | ||
} | ||
|
||
scriptsToInject.forEach((scriptToInject) => { | ||
|
@@ -419,7 +401,7 @@ PreviewFrame.propTypes = { | |
textOutput: PropTypes.bool.isRequired, | ||
gridOutput: PropTypes.bool.isRequired, | ||
soundOutput: PropTypes.bool.isRequired, | ||
setTextOutput: PropTypes.func.isRequired, | ||
// setTextOutput: PropTypes.func.isRequired, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should this be removed rather than commented out? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. |
||
htmlFile: PropTypes.shape({ | ||
content: PropTypes.string.isRequired | ||
}).isRequired, | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,15 +13,19 @@ const defaultHTML = | |
`<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.2.1/jquery.min.js"></script> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is this including jquery? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we will clear this out - our library has dependencies on jquery but we will make them vanilla javascript. |
||
<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/0.5.11/p5.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/0.5.11/addons/p5.dom.min.js"></script> | ||
<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/0.5.11/addons/p5.sound.min.js"></script> | ||
<link rel="stylesheet" type="text/css" href="style.css"> | ||
<meta charset="utf-8" /> | ||
|
||
</head> | ||
<body> | ||
<section aria-label='accessible output' id='accessible-outputs'></section> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than add the accessibility elements and css to the default html file, i think it would be better to inject them in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @catarak - I think some of the questions we had for you were deleted after we made the new commits :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. got it. should it be added by default or only if accessibility features are turned on? and also, you want the script to point to a specific release of the library? or a specific commit? or to the latest on master? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we tried to add it only if accessibility features are added but there was a problem? @lm-n ? |
||
<script src="sketch.js"></script> | ||
</body> | ||
<div id='accessibility-library'></div> | ||
</body> | ||
</html> | ||
`; | ||
|
||
|
@@ -30,6 +34,15 @@ const defaultCSS = | |
margin: 0; | ||
padding: 0; | ||
} | ||
|
||
#accessible-outputs { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i think this is the last thing, but this CSS should also get injected by the library! |
||
position: absolute; | ||
left: -10000px; | ||
top: auto; | ||
width: 1px; | ||
height: 1px; | ||
overflow: hidden; | ||
} | ||
`; | ||
|
||
const initialState = () => { | ||
|
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.
rather than link to the accessibility using raw git, i think it would be better to add the library as a dependency in
package.json
, then import the built script using webpack.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.
cc @lm-n, @CleezyITP - We have been talking about this one, we would like the library to be a part of the HTML code as a CDN so that when a user shares their code (say, send a zip to someone, it will include the library. This would make sharing more accessible. What do you think?
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.
@catarak - was wondering what your thoughts on this are?