Skip to content
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

Screen descriptionContent should use helpText for consistency #989

Closed
jessegreenberg opened this issue Nov 5, 2024 · 10 comments
Closed

Comments

@jessegreenberg
Copy link
Contributor

This option is used to set the help text of the home screen button for a particular screen

      // pdom/voicing - The description that is used when interacting with screen icons/buttons in joist (and home screen).
      // This is often a full but short sentence with a period at the end of it. This is also used for voicing this screen
      // in the home screen.
      descriptionContent: null,

We should change it to use helpText to for better consistency as we move toward using "accessible name" and "help text" throughout code components.

Came up while reviewing phetsims/models-of-the-hydrogen-atom#67 with @kathy-phet and @terracoda.

@jessegreenberg
Copy link
Contributor Author

In phetsims/models-of-the-hydrogen-atom#73 (comment) @pixelzoom recommended

- descriptionContent?: PDOMValueType | null;
+ // Help text that will be added to the Home screen button and navigation bar button for this screen.
+ screenButtonsHelpText?: PDOMValueType | null;

pixelzoom added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Nov 5, 2024
@pixelzoom
Copy link
Contributor

@jessegreenberg When the API is changed, feel free to update MOTHA. I added 2 TODOs that link to this issue.

jessegreenberg added a commit to phetsims/area-model-common that referenced this issue Nov 5, 2024
jessegreenberg added a commit to phetsims/greenhouse-effect that referenced this issue Nov 5, 2024
jessegreenberg added a commit to phetsims/models-of-the-hydrogen-atom that referenced this issue Nov 5, 2024
jessegreenberg added a commit to phetsims/ratio-and-proportion that referenced this issue Nov 5, 2024
@jessegreenberg
Copy link
Contributor Author

OK, this is done. I updated usages, including the two in MOTHA and removed the TODOs. I also updated string keys under the a11y key based on a comment about how they were confusing before in phetsims/models-of-the-hydrogen-atom#73 (comment).

Here is the main joist commit: e3366b8 with some cleanup in 07dea6d.

@pixelzoom would you mind reviewing this?

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 6, 2024

Commits look good.

Should the default helpText match the default for the Home Screen button? That is "Go to {{screenName}} Screen." rather than just "{{screenName}} Screen". See screenshot below.

screenshot_3578

@jessegreenberg
Copy link
Contributor Author

Yes, and that is a better default. Anything else for this?

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 8, 2024

Looking good!

In joist-strings_en.json:

    "home": {
      "value": "Home"
    },

    "homeScreenDescription": {
      "value": "Go to Home Screen."
    },

    "goToScreenPattern": {
      "value": "Go to {{name}} Screen."
    },

Should homeScreenDescription be replaced with goToScreenPattern? This would ensure consistency going forward.

@pixelzoom pixelzoom assigned jessegreenberg and unassigned pixelzoom Nov 8, 2024
@terracoda
Copy link
Contributor

@pixelzoom and @jessegreenberg, I also like "Go to" as a default, and hopefully folks will design screen button help text that's more directly relevant. Great default, though!

@jessegreenberg
Copy link
Contributor Author

Thanks @terracoda, sounds good.

Good point @pixelzoom, done in the above commit. Can you please review?

@pixelzoom
Copy link
Contributor

pixelzoom commented Nov 13, 2024

Looks good, but I'm not sure if it's safe to totally remove a string from joist-strings_en.json (homeScreenDescription in this case). @jessegreenberg please verify with @jbphet.

@jessegreenberg
Copy link
Contributor Author

Thanks for reviewing. It is safe to remove strings under the a11y key, I confirmed this with @jbphet for another issue a few weeks ago.

Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants