Rename "Interactive Descriptions" to "Interactive Description" across the project

phetsims / a11y-research

a repository to track PhETs research into accessibility, or "a11y" for short

MIT License

3 stars 0 forks source link

Rename "Interactive Descriptions" to "Interactive Description" across the project #157

Closed zepumph closed 3 years ago

zepumph commented 4 years ago

I hear @terracoda over in https://github.com/phetsims/a11y-research/issues/145#issuecomment-614259137. We should make sure to fix this across the project

Currently there are 23 usages of interactiveDescriptions, and 19 usages of "interactive descriptions" (case insensitive).

@jessegreenberg whoever get's this this first.

zepumph commented 3 years ago

I did a large refactor. I don't feel like this needs to end up in bad text or anything, because I feel like the convention will be followed. @jessegreenberg do you feel differently? Anything else here?

jbphet commented 3 years ago

I believe that this rename may have created backwards-compatibility issues with release branches. I just did a checkout of the 1.2 branch of atomic-interactions, tried to load phetmarks, and got the following error:

The full path for the abbreviated part in the screenshot is:

http://localhost:8080/perennial/data/interactive-descriptions

It can't find it because the file has been renamed. I think this rename may need to be reconsidered or some way of supporting backwards compatibility needs to be worked out.

BTW, I removed the ready-for-review label.

jbphet commented 3 years ago

I labeled this as "high-priority" because it's messing with my ability to test release branches, and I assume others are in the same boat or will be shortly.

Two things to note:

a workaround for this if trying to test a release branch is to change the line that says url: '../perennial/data/interactive-descriptions' to url: '../perennial/data/interactive-description' (just drop the 's') in phetmarks.js
one solution would be to do a maintenance release to make this work, but I suspect that probably isn't worth the effort

jessegreenberg commented 3 years ago

Overall the rename looks good to me.

For the compatibility issue with old phetmarks I am not sure. "Interactive Description" is being used publicly now on the website to brand this set of features so I think we should keep the rename.

The file rename in phetmarks could be relatively painless as maintenance releases go, maybe we should commit to that. @zepumph what do you think, do you see another way?

jessegreenberg commented 3 years ago

@zepumph and I discussed - @zepumph had a thought that the fixing commit for this could be applied to all release branches without actually publishing them. This would mean that we would go all the way up to updateDependencies in the MR process then leave commits unpublished.. Unfortunatley, these unpubilshed commit would then get flagged for the next batch of maintenance releases when someone runs checkBranchStatus. Is there a way to make a commit to a release branch and flag it as acceptable?

Otherwise, we agreed that a batch of maintenance releases seems like the best way forward. I will check in with @jonathanolson about this.

jonathanolson commented 3 years ago

Unfortunatley, these unpubilshed commit would then get flagged for the next batch of maintenance releases when someone runs checkBranchStatus. Is there a way to make a commit to a release branch and flag it as acceptable?

Please feel free to do this! I'm not concerned about that at all.

jessegreenberg commented 3 years ago

OK thanks @jonathanolson - I will go ahead with this then.

jessegreenberg commented 3 years ago

Dependencies have been updated for release branches in https://github.com/phetsims/phetmarks/issues/48. The only exception a phet-io specific release branch of molarity which will be taken care of separately in that issue.

I verified that phetmarks is launchable from atomic-interactions 1.2 SHAs again. @jbphet reassigning to you to verify if you wish, otherwise I think this can be closed.

jbphet commented 3 years ago

I tested this by checkout out the 1.2 branch of atomic-interactions and testing phetmarks, and it worked fine. Thanks for the fix @jessegreenberg and @zepumph. Closing.