Issue 9100

[Jextic]

Resolves #9100

Changes:
API Changes:

• Changed describe function from describe(text, display) to describe(text, langOrDisplay, display). Similar changes were made to describeElement function. langOrDisplay is optional and can accept either a lang attribute (such as 'en') or display (FALLBACK or LABEL). This also allows it to be backwards compatible.
• Helper function _parseOptions was made to help determine what optional parameters are used.
• Lang attribute is injected into inner so to allow sketch creators to have multiple sketches with multiple lang attributes (otherwise, one lang attribute would override all the others).
• Examples of possible describe combinations (describeElement also applies):

describe('text')
describe('text', LABEL)
describe('text', 'en')
describe('text', 'en', LABEL)
describe('text', LABEL, 'en')

Order of optional parameters doesn't matter so that it's beginner friendly.

Manual Test Page:

• HTML page and sketch (global) was created to include the test cases from the issue plus some more cases that I used to make sure my implementation worked.

Unit Tests and JSDoc Updates:

• Unit tests were added to test the parameter combinations for describe (3 unit tests) and for describeElement (3 unit tests)
• @params were adjusted to include the optional langOrDisplay parameter. how the langOrDisplay parameter works with the optional display parameter is also included.

Screenshots of the change:

screen-reader-lang-test.mp4

I tried my best to showcase Windows Narrator working reading the describe() functions. The behavior of the screen reader works as expected for each of the tests (5 sketch tests for the parameter combinations and 1 sketch test to show that without the lang attribute, the screen reader skips native script text).

The failed test is typography test which still failed in the main branch.

PR Checklist

• npm run lint passes
• Inline reference is included / updated
• Unit tests are included / updated

[p5-bot]

Continuous Release

CDN link

• https://raw.esm.sh/pr/p5@6cfaf8a/lib/p5.min.js

Published Packages

• https://pkg.pr.new/p5@6cfaf8a

Commit hash: 6cfaf8a

Previous deployments

---

This is an automated message.

[ksen0]

Do we have this pattern anywhere else? I'm not remembering anything off the top of my head. But it's quite a challenging pattern. cc @perminder-17 sorry if it was already discussed somewhere and I missed it, but my thoughts below.

From your examples:

describe('text')
describe('text', LABEL)
describe('text', 'en')
describe('text', 'en', LABEL)
describe('text', LABEL, 'en')

So the second one is lang or display also? Maybe it should be parameter1 and parameter2 and either can be lang or display but to be honest if we dont already have precedent for this kind of approach, I am a little worried about it being very confusing for learners, and resulting in outdated documentation for contributors.

[ksen0]

To avoid duplication of this code, you can use decorators. For example, here how we use decorators for v ector validation, where very similar code runs for each of the vector binary operations: https://github.com/processing/p5.js/blob/main/src/math/patch-vector.js we wanted to avoid copy-pasting the same code in 5 places, and we used that decorator pattern instead. Firendly Errors also use decorators. Please do try using this pattern to avoid this option interpretation (even though in your case its just 2 places); the decorators API was recently added so don't hesitate to @ me on discord for help with this one.

[Jextic]

Thanks @ksen0 for the feedback!

I thought it would be beginner friendly to make it, like you said, describe('text', parameter1, parameter2) where the parameters are optional and the parameters can be display or a lang. However, if it is inconsistent with all the other p5 functions, I can revert it back to the original implementation where it has the strict format describe('text', display, lang). That way, users must include display if they want to add a lang. They can just pass FALLBACK as the display and type the lang right after. This would still make the describe() functions backwards compatible with older sketches

As for the decorator API, am I understanding this correctly?
A decorator pattern (function) can be used to intercept the parameters. In the vector class, it's used to check if the argument is valid and convert it to an array so that it uses a single standard format (the array) that makes it easier to work with. I can use a similar decorator pattern to intercept the describe and describeElement optional parameters, lang and display, to handle those parameters instead using the option parsing approach.

I'd like to confirm the preferred syntax of the describe/describeElement functions before working on a decorator for the optional parameters.

[ksen0]

@Jextic good question, I made a guide here #9118 please feel free to comment there if any thing is unclear, or @ me in discord. The decorators are new since 2.3.0 so your feedback will also help make this more usable by future contributors!

Regarding parameters, as it's an API style decision, I will ask on Discord the other maintainers (you'll see it in the a11y channel). My opinion is that describe('text', parameter1, parameter2) is more beginner-friendly and backwards-compatible, but it's possible this pattern is too unusual. Feedback also welcome below form anyone not on Discord