Accessibility - Transition from interceptor to accessibility library #508

lm-n · 2018-01-30T09:52:00Z

added accessible elements to iframe using accessibility library
removed interceptor
added aria labels
added css to iframe
focus on iframe when play is pressed

…ility library

Transition from Interceptor to Accessibility Library

* remove git modules

catarak · 2018-01-30T20:20:14Z

yay this is very exciting! in README.md, in the installation instructions, there's a section about the submodule setup—could you remove those parts in this PR as well? thanks!

catarak

i think you need to do something to remove the submodule besides deleting .gitmodules—when i look at the accessibility branch on github the submodule is still there.

catarak · 2018-01-31T17:51:12Z

client/modules/IDE/components/PreviewFrame.jsx

-      }
-      scriptsToInject = scriptsToInject.concat(interceptorScripts);
+    const accessiblelib = sketchDoc.createElement('script');
+    accessiblelib.setAttribute('src', 'https://cdn.rawgit.com/MathuraMG/p5-accessibility/e856365c/dist/p5-accessibility.js');


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.

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?

@catarak - was wondering what your thoughts on this are?

catarak · 2018-01-31T17:51:18Z

client/modules/IDE/reducers/files.js

@@ -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>


why is this including jquery?

we will clear this out - our library has dependencies on jquery but we will make them vanilla javascript.

catarak · 2018-01-31T17:52:16Z

client/modules/IDE/components/PreviewFrame.jsx

@@ -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,


should this be removed rather than commented out?

MathuraMG · 2018-02-01T16:13:16Z

@catarak - We will fix the above asap.
Also, a quick update. There is an issue with the accessibility library that we found yesterday. We will fix that and then get back on this PR, so that we have the right version of the library put in. Is that ok?
Thanks!

@lm-n

catarak · 2018-02-01T21:36:41Z

no worries, that is of course okay! this brings up an important point, which is how we want to connect the version of the accessibility library to the editor in the package.json, so that when you run npm install in the editor, it doesn't install the latest commit, but a stable commit— we could 1. add a commit sha, or 2. tag a release version, using semantic versioning. (1) could come first and then (2) could come later.

* removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn

catarak

looking great, a couple more things left to iron out though! also, question, is there a git command to run to delete a submodule? or do you have to just delete the directory? i'm worried we'll merge this, and then someone will accidentally check in the submodule directory.

catarak · 2018-02-12T17:10:38Z

client/modules/IDE/reducers/files.js

  </head>
  <body>
+    <section aria-label='accessible output' id='accessible-outputs'></section>


rather than add the accessibility elements and css to the default html file, i think it would be better to inject them in the PreviewFrame.jsx file, in the function injectLocalFiles. As it is now, a user could accidentally delete these elements from the HTML and then the accessibility features would be broken.

@catarak - I think some of the questions we had for you were deleted after we made the new commits :)
@lm-n and myself were thinking it should be a part of the HTML (at the risk of being deleted) , the same way as p5-dom or p5-sound. This way if someone decides to share their code, they can share or download it with the accessibility lib and so the code stays accessible beyond the editor. What do you think about this?

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?

I think we tried to add it only if accessibility features are added but there was a problem? @lm-n ?

catarak · 2018-02-12T17:12:36Z

client/modules/IDE/components/PreviewFrame.jsx

-      }
-      scriptsToInject = scriptsToInject.concat(interceptorScripts);
+    const accessiblelib = sketchDoc.createElement('script');
+    accessiblelib.setAttribute('src', 'https://cdn.rawgit.com/MathuraMG/p5-accessibility/9cb5bd0b/dist/p5-accessibility.js');


i still would prefer to link this file by putting it in the package.json and importing the build with webpack!

@MathuraMG and i talked through this and decided it would be best to address this in the future ✨

* removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js

lm-n · 2018-02-15T15:15:50Z

@catarak I think both should work. When I move into the accessibility branch the submodule directory does not exist anymore: when writing git submodule it doesn't return anything, if the submodule directory/the submodule was still there when writing git submodule it would return: -62938f2e9cbe1c83a9c1b702a9deccb11a452170 static/p5-interceptor It can be done also with git submodule deinit -f [submodule name] rm -rf [submodule directory] git rm -f [submodule name] @MathuraMG

catarak · 2018-02-15T19:33:28Z

client/modules/IDE/reducers/files.js

@@ -30,6 +31,15 @@ const defaultCSS =
  margin: 0;
  padding: 0;
 }
+
+#accessible-outputs {


i think this is the last thing, but this CSS should also get injected by the library!

* removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js * moved accessible output css

catarak · 2018-02-20T17:26:51Z

client/modules/IDE/components/PreviewFrame.jsx

@@ -207,6 +207,12 @@ class PreviewFrame extends React.Component {
    accessiblelib.setAttribute('src', 'https://cdn.rawgit.com/MathuraMG/p5-accessibility/9cb5bd0b/dist/p5-accessibility.js');
    const accessibleOutputs = sketchDoc.createElement('section');


should these accessible outputs be created in the library itself? this is necessary for the library to work, right?

The reason we don't create the sections in the library is because the section is currently the trigger to which output the user wants. So in this case we create that section when they select the option in preferences.

@lm-n

got it. would it instead make sense to make some boolean for the library instead that enables/disables the accessible output? then, in the future, if this ever needs to change you can change it in the accessibility library rather than here as well.

Sorry, I didn't get what you mean by ' boolean for the library'? :)
The reason we are having sections now is so that beyond the editor the user can determine where they want the output in their program.

got it! what i mean is maybe there's a script that initializes the library, rather than just appending to an HTML element if it's there. for example, one includes the accessibility library, and then includes a script:

var accessibility = new p5.Accessibility(<reference to the HTML element you want to append to>);

or it could create an HTML element to append to by default. My inspiration for this is dat.gui. my instinct is that it is not the best that including an html element determines whether or not the library is included.

sure! should i merge this in then?

sounds good!

Here are the issues on the library repo: processing/p5.accessibility#13 processing/p5.accessibility#12 We'll follow up in the next few days.

@catarak Yes! That sounds good!

great, will do ⭐️

catarak · 2018-02-22T21:47:10Z

just another thought that there's an open issue for, #137: right now, accessibility features won't be added to the full screen view or embed view. and if a user downloads a sketch it won't be included. open to thoughts!

* Remove gitmodule (processing#509) * remove git modules * removed submodule and replaced interceptor for library * removed submodule and replaced interceptor for library (processing#510) * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * Fixes processing#508 (processing#539) * removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js * Fixes processing#508 (processing#545) * removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js * moved accessible output css * removed css, added aria-label, preliminary cdn update * removed section from iframe * updated cdn

* Change accessibility example links * added library to iframe * changed preview to add accessible elements to iframe * add library only when accesible output is seleceted * focus on iframe when plaing * css * deleted accessibleOutput.jsx and edited IDEView to integrate accessibility library * deleted comments * fix package * Moved CSS to library and removed section from file.js (#2) * Remove gitmodule (#509) * remove git modules * removed submodule and replaced interceptor for library * removed submodule and replaced interceptor for library (#510) * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * Fixes #508 (#539) * removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js * Fixes #508 (#545) * removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn * remove empty divs from files.js * fix merge error * remove empty divs from files.js * moved accessible output css * removed css, added aria-label, preliminary cdn update * removed section from iframe * updated cdn * add autofocus; remove CSS

Luis Morales-Navarro and others added 9 commits December 22, 2017 11:47

added library to iframe

344e50e

changed preview to add accessible elements to iframe

cff4819

add library only when accesible output is seleceted

dcda574

focus on iframe when plaing

cf76989

css

1ae2c6f

deleted accessibleOutput.jsx and edited IDEView to integrate accessib…

7097808

…ility library

deleted comments

d98e700

Merge pull request #506 from lm-n/accessibilitylib

e510b8f

Transition from Interceptor to Accessibility Library

Remove gitmodule (#509)

a2f70a4

* remove git modules

removed submodule and replaced interceptor for library (#510)

c33d68c

catarak reviewed Jan 31, 2018

View reviewed changes

catarak mentioned this pull request Feb 7, 2018

Add a hidden menu/button for turning on accessibility features #504

Open

3 tasks

lm-n mentioned this pull request Feb 12, 2018

Fixes #508 #539

Merged

Fixes #508 (#539)

bb846c2

* removed submodule and replaced interceptor for library * deleted comments * deleted jquery * deleted interceptor folder * delete interceptor * added jquery * removed jquery and updated accessible library cdn

catarak requested changes Feb 12, 2018

View reviewed changes

fix merge conflicts

2e97dd2

catarak requested changes Feb 15, 2018

View reviewed changes

lm-n mentioned this pull request Feb 18, 2018

Fixes #508 #548

Merged

catarak requested changes Feb 20, 2018

View reviewed changes

lm-n mentioned this pull request Feb 22, 2018

Add library initializer processing/p5.accessibility#13

Open

catarak merged commit 24b0be6 into master Feb 22, 2018

catarak deleted the accessibility branch May 21, 2020 17:22

lindapaiste mentioned this pull request Apr 23, 2023

Turning on accessible canvas settings causes sketches to break #2188

Closed

		@@ -207,6 +207,12 @@ class PreviewFrame extends React.Component {
		accessiblelib.setAttribute('src', 'https://cdn.rawgit.com/MathuraMG/p5-accessibility/9cb5bd0b/dist/p5-accessibility.js');
		const accessibleOutputs = sketchDoc.createElement('section');

Uh oh!

Accessibility - Transition from interceptor to accessibility library #508

Accessibility - Transition from interceptor to accessibility library #508

Uh oh!

Conversation

lm-n commented Jan 30, 2018

Uh oh!

catarak commented Jan 30, 2018

Uh oh!

catarak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MathuraMG commented Feb 1, 2018

Uh oh!

catarak commented Feb 1, 2018

Uh oh!

catarak left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MathuraMG Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lm-n commented Feb 15, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lm-n Feb 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

catarak commented Feb 22, 2018

Uh oh!

Uh oh!

MathuraMG Feb 14, 2018 •

edited

Loading

lm-n commented Feb 15, 2018 •

edited

Loading

lm-n Feb 22, 2018 •

edited

Loading