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

Guidelines for range related properties #1279

Merged
merged 63 commits into from
Jul 7, 2020

Conversation

zcorpan
Copy link
Member

@zcorpan zcorpan commented Dec 3, 2019

Replaces #1000, changed to be a branch in this repo so it's easier to collaborate.

Preview Link

View the new section the compare branch using RawGitHack


Preview | Diff

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed range related properties.

The full IRC log of that discussion <carmacleod> TOPIC: range related properties
<carmacleod> mck: did we resolve the questions about undetermined value? (i.e. min and max undefined)
<carmacleod> simon: spinbutton is an example that allows undefined min and max
<carmacleod> mck: progressbar too
<carmacleod> mck: we don't have an apg example of something that doesn't have a defined range
<Jemma> for aria widget status, are you starting from here, https://www.w3.org/TR/wai-aria-1.1/#attrs_widgets?
<zcorpan> https://pr-preview.s3.amazonaws.com//pull/1279.html#range_related_properties
<zcorpan> GitHub: https://github.com//pull/1279
<Jemma> Communicating Value for Range Widgets
<Jemma> aria-valuetext Defines a description of the value of the element.
<carmacleod> simon: I added an example of an undetermined progressbar
<carmacleod> jon: need more on when to use aria-valuetext
<carmacleod> jon: how should you think about valuemin and valuemax if you have valuetext
<carmacleod> jon: i.e. if we have low, medium, high, do I put in 1 for min and 3 for max?
<carmacleod> simon: aria spec says you can _also_ use valuetext as well as min and max, so unless spec changes, we need to have min and max
<carmacleod> simon: if it's required for the role
<carmacleod> jon: if it's a price range, screen readers only say the text
<carmacleod> mck: if I want to know the range, I type home and end key to find min and max
<Jemma> +q
<Jemma> https://pr-preview.s3.amazonaws.com//pull/1279.html#range_related_properties_using_aria-valuemin_and_aria-valuemax
<carmacleod> mck: some will communicate percentage and some will communicate actual values (I think spinbuttons say value, and sliders say percentage)
<carmacleod> ack jem
<Jemma> https://rawcdn.githack.com/w3c/aria-practices/1f7d940ca9872ff2f240c65ce4520338fbdbd3c6/examples/meter/meter.html
<carmacleod> simon: for progressbar, min and max default to 0 and 100; for meter, authors are required to provide min and max

@zcorpan
Copy link
Member Author

zcorpan commented Dec 4, 2019

@a11ydoer found the table for aria-valuemin and aria-valuemax confusing. Related to this, I've filed an issue for the ARIA spec about the meter role: w3c/aria#1123

@zcorpan
Copy link
Member Author

zcorpan commented Dec 4, 2019

I have also tried to clarify the table itself, by changing "none" to "no default", and the table heading "Required?" to "Are aria-valuemin and aria-valuemax required?".

@zcorpan
Copy link
Member Author

zcorpan commented Dec 4, 2019

I'm not certain what to say about the purpose of aria-valuemin and aria-valuemax when aria-valuetext is used. Any suggestions?

@jongund
Copy link
Contributor

jongund commented Dec 10, 2019

@mcking65 @zcorpan I like the battery example, but does it need to include aria-valuemin, aria-valuemax, and aria-valuenow if the only information spoken is aria-valuetext?

Also what is the guidance for some thing like a range of costs and aria-valuetext?
For example, a price range between $50-$100, should aria-valuetext be used to include a aria-value text="75 dollars", instead of just aria-valuenow="75"? Is either good? or is one better?

aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
@zcorpan
Copy link
Member Author

zcorpan commented Jan 9, 2020

Thanks, @carmacleod, I've applied your suggestions.

@jongund I think your question is the topic of w3c/aria#1128

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed Guidance section - range related properties.

The full IRC log of that discussion <MarkMccarthy> TOPIC: Guidance section - range related properties
<MarkMccarthy> Matt_King: i'm probably just going to push some commits. have 4 minor editorial changes to make
<Jemma> https://github.com//pull/1279
<MarkMccarthy> Matt_King: one thing though, we have a section abour progress, scroll bars, spinbuttons and a couple more in section 8.4, but not meters.
<carmacleod> github: https://github.com//pull/1279
<MarkMccarthy> Matt_King: i wonder if someone might make a similar section for meter?
<MarkMccarthy> s/in section 8.4/sections for each
<MarkMccarthy> carmacleod: i can't, have some work stuff that has to come first.
<MarkMccarthy> Matt_King: understandable.
<MarkMccarthy> Matt_King: we could merge without, just seems like an oversight to me
<MarkMccarthy> Jemma: i can do it if I have more info, since I did the meter example
<MarkMccarthy> Matt_King: sure, if you want to draft it. essentially, if you look at section 8.6, you could copy most of that and replace phrases where appropriate
<MarkMccarthy> Matt_King: you'd have to do inline example code though
<MarkMccarthy> Matt_King: yeah, literally can be copied as long as it's made relevant to meters.
<MarkMccarthy> Jemma: oaky
<MarkMccarthy> s/oaky/okay
<MarkMccarthy> Matt_King: and needs to be alphabetical, so it'd come before progressbar and after valuetext
<MarkMccarthy> Matt_King: there's a branch here you can use, too. no worries
<MarkMccarthy> Jemma: got it
<MarkMccarthy> Matt_King: any other comments? I'd like folx to read this in the coming week. i'll work w/ you on final edits Jemma. I'd like to merge this next week if possible.
<MarkMccarthy> Matt_King: if there's common questions we haven't answered; issues authors run into commonly; other things that aren't addressed well or at all; all good to know.
<MarkMccarthy> Matt_King: jongund, have you read that yet?
<MarkMccarthy> jongund: looking at it now
<MarkMccarthy> Matt_King: you're probably someone who's very in touch with people who stumble
<MarkMccarthy> jongund: i stumble! [laughs]
<MarkMccarthy> [group laughter]
<MarkMccarthy> carmacleod: your resiliance is impressive, Jon! :)

@jongund
Copy link
Contributor

jongund commented Feb 19, 2020

I like the table for default and required values, but I think it should also include aria-valuenow, and be formatted with two rows of headers to make it visually easier to read.

  1. The top row should have colspans for "Default Values" and "Required". The
  2. The scond row of headers role, aria-valuemin, aria-valuemax, aria-valuenow, aria-valuemin, aria-valuemax and aria-valuenow
  3. the following roles could use none when there is no default value
  4. The required columns could be yes and no.

Including the aria-valuenow in the table would provide one place to see all this information, and the descriptions of each property will reinforce the information in the table.

Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment about expanding the table of default and required values to include aria-valuenow.

The table would be better in the previous section.

The example in 8.5 it would be good to include a comment in the sample code that the scrollbar is for showing more or less of the digits of the constant PI.

aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
@mcking65
Copy link
Contributor

@a11ydoer, thank you for the meter section; it is looking good.

@carmacleod, thank you for the suggestions; they are all good as well.

I think the meter and spin button sections should have links back to the pattern like in slider.

I am curious if others would like to see the table of requirements by role expanded to include valuenow as Jon has suggested? I don't recall if there is a default calculation of valuenow for any of these roles. If not, we may not want a default column for valuenow. However, we could certainly have a required yes/no column for it.

@carmacleod
Copy link
Contributor

@mcking65

I am curious if others would like to see the table of requirements by role expanded to include valuenow as Jon has suggested?

Yes, I think that would make things clearer, and all in one place.

I don't recall if there is a default calculation of valuenow for any of these roles.

I believe the only role that doesn't require valuenow is spinbutton.

@carmacleod
Copy link
Contributor

@mcking65 The latest commit does the following:

  • restructure the table in 8.1 as suggested by @jongund (add valuenow; group columns under "Default Values" and "Required")
  • collapse 8.2 (valuenow) into 8.1 (min & max) to make a single 8.1 for min, max and valuenow
  • update meter based on PR revise meter authoring advice aria#1129 (min, max are supported and default to 0, 100; valuenow is required)

Preview the new section 8.1.

@jnurthen - ping on w3c/aria#1129 for 1.2 so that APG and ARIA 1.2 are in sync

@carmacleod
Copy link
Contributor

The table is correct - I triple-checked when I modified it. Agree that the prose does need to be updated in places to match the table.

Looking at the table again now, though, I wonder if it would be clearer to delete the 3 "Required" columns, and instead, just use the word "Required" in the valuenow cells, maybe with an asterisk footnote below the table reminding authors that they need to provide a value. I also wonder if we should have 2 rows for progressbar (one for indeterminate), or maybe use another asterisk footnote to explain that a value is needed unless it's indeterminate? Similarly, spinbutton needs valuenow set if it has a value, so maybe we should point that out?

For example, the "Default Values Table" could be rewritten like this:

Role aria-valuemin aria-valuemax aria-valuenow
meter 0 100 Required *
progressbar 0 100 None **
scrollbar 0 100 Required *
separator (if focusable) 0 100 Required *
slider 0 100 Required *
spinbutton None None None ***

* authors need to provide a value
** authors need to provide a value unless the progressbar is indeterminate
*** authors need to provide a value if the spinbutton has a value

Do people think this is clearer or more complicated? (note that it does contain more information in 4 columns than the current table contains in 7 columns...)

@jongund
Copy link
Contributor

jongund commented Mar 9, 2020

How about something like this:

Role aria-valuemin (default) aria-valuemax (default) aria-valuemin (required) aria-valuemax (required) aria-valuenow (required)
meter 0 100 When minimum value is not 0 When maximum value is not 100 Yes
progressbar 0 100 When minimum value is not 0 When maximum value is not 100 Yes, unless indeterminate
scrollbar 0 100 When minimum value is not 0 When maximum value is not 100 Yes
separator (if focusable) 0 100 When minimum value is not 0 When maximum value is not 100 Yes
slider 0 100 When minimum value is not 0 When maximum value is not 100 Yes
spinbutton None None minimum value if known maximum value if known Yes, if known

@carmacleod
Copy link
Contributor

carmacleod commented Mar 17, 2020

I like @jongund's latest table, except that I think the columns for aria-valuemin (required) and aria-valuemax (required) add complexity that is not strictly necessary, and could be confusing. Can we delete those two columns? I think authors know that if they don't want an attribute's default value, they need to set it to something else.

Not sure if "current value" for valuenow might be confusing? i.e. could be confused as follows:
"it's required, but it has a default of 'current value'?" Maybe use "authors must provide value" instead?

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Mar 17, 2020

The ARIA Authoring Practices (APG) Task Force just discussed Guidance on range-related properties.

The full IRC log of that discussion <carmacleod> TOPIC: Guidance on range-related properties
<carmacleod> github: https://github.com//pull/1279
<carmacleod> mck: we are really close on this - need to make descriptions line up with info in table
<carmacleod> mck: maybe change "current value" to "yes" (i.e. for "required - yes")
<carmacleod> jongund: I updated the table to have "Yes"

@mcking65
Copy link
Contributor

mcking65 commented Mar 31, 2020

To wrap this up, we need commits to this branch that:

  • Update table in section with id="range_related_properties_using_aria-valuemin_aria-valuemax_and_aria-valuenow" (8.1 in the rawgithack view) to match table in Jon's comment above
  • Edit text for min/max/now in sections 8.3 through 8.7 where the text does not match the requirements in the table.
  • In the meter section (8.3), add a sentence with a link to the meter pattern similar to the sentence in the slider section that links to the slider pattern.
  • In the spin button section (8.7), add a sentence with a link to the spin button pattern similar to the sentence in the slider section that links to the slider pattern.

@a11ydoer a11ydoer self-requested a review March 31, 2020 18:36
@a11ydoer
Copy link
Contributor

a11ydoer commented Apr 5, 2020

@jongund @mcking65 @carmacleod I created PR for the range related property. I appreciate if you can have a look for final feeback. Thanks, Jemma

Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change 1

"The meter role has the default value 0 for aria-valuemin, 100 for aria-valuemax. When the meter role does not provide default range values from 0 to 100, aria-valuemin and aria-valuemax property values are required. Also aria-valuenow range property is required for meter. Detailed description of the meter role can be found in the meter design pattern. "

I think the statements of defining the minimum and maximum values should be the similar for all the range related roles.

I suggest some changes to the second and third sentences:

The meter role has the default values 0 for aria-valuemin and 100 for aria-valuemax and only need to be set when the actual minimum and maximum values are different. The aria-valuenow property is required for meter and the author needs to make sure aria-valuenow is within the minimum and maximum values . Detailed description of the meter role can be found in the meter design pattern. "

Change 2

Most of the examples set the aria-valuemin and aria-valuemax to the default values, wouldn't it be better in these examples to eliminate setting them and have a comment that says the values are using the default values of 0 and 100.

<span id="cpu_usage_label">CPU usage</span>
<div role="meter"
   // The CPU usage uses the default values of 0 and 100 for aria-valuemin and aria-valumax
      aria-valuenow="90"
      aria-labelledby="cpu_usage_label">
</div>

For the meter role we could have an additional code example using a different range, for example a PH meter.

<span id="ph_meter_label">PH</span>
<div role="meter"
      aria-valuenow="1"
      aria-valuemin="7"
      aria-valuemax="14"
      aria-labelledby="ph_meter_label">
</div>

@mcking65 mcking65 force-pushed the zcorpan/range-related-properties-2 branch from 454b04d to 864db2b Compare July 1, 2020 22:31
@mcking65
Copy link
Contributor

mcking65 commented Jul 1, 2020

@jongund, @a11ydoer, @carmacleod, I've completed final editorial review and revisions. I believe this is ready to merge. However, I'd like to have some approving reviews, especially from Jon since your last review was requesting changes before merge.

Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to request changes, but there are 3 code examples (progressbar, 2 spinbutton) where a required label is missing.
There's also 1 place (scrollbar) where a discretionary label might be useful (need a second opinion on that one).
And there's an editorial nit (missing "a").

Other than that, this looks great!

aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
aria-practices.html Outdated Show resolved Hide resolved
Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In section 8.7 there is the following statement:
"If there are minimum and maximum allowed values, set the aria-valuemin and aria-valuemax properties."

What should be done if there is no minimum or maximum values?
Omitting the attributes gives you the default values of 0 and/or 100.

Are progress bars the only widget that can be indeterminate, it is the only one mentioned in the ARIA Spec?

If so maybe all the information related to indeterminate state should be moved to the progress bar section and removed form the other sections of the range widget.

Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the Section 8.1 on range guidance there seems to be a disconnect for ranges that are indeterminate and using default values for range attributes. The guidance for indeterminate is to not include aria-valuemax or aria-valuemin, but we also say when one or both of these attributed are not included that the default values of 0 and 100 will be used. So which is it, default values or indeterminate? Maybe it should be when the aria-valuenow attribute is missing or empty it should be the way to author identifies an indeterminate state.

@mcking65
Copy link
Contributor

mcking65 commented Jul 7, 2020

@jongund wrote:

In section 8.7 there is the following statement:
"If there are minimum and maximum allowed values, set the aria-valuemin and aria-valuemax properties."

What should be done if there is no minimum or maximum values?
Omitting the attributes gives you the default values of 0 and/or 100.

Section 8.7 says:

The aria-valuemin and aria-valuemax properties are used only when a spinbutton has a defined range. They do not have default values, so if they are not specified, range limits will not be exposed to assistive technologies.

@jongund wrote:

Are progress bars the only widget that can be indeterminate, it is the only one mentioned in the ARIA Spec?

If so maybe all the information related to indeterminate state should be moved to the progress bar section and removed form the other sections of the range widget.

Section 8.4 is about progressbar, and it says the following:

The aria-valuemin and aria-valuemax properties only need to be set for the progressbar role when the progress bar's minimum is not 0 or the maximum value is not 100. The aria-valuenow property needs to be set for a progressbar if its value is known (e.g. not indeterminate). If the aria-valuenow property is set, the author needs to make sure it is within the minimum and maximum values.

So, while the current value may be indeterminate, the limits of the range are never indeterminate.

There are only 2 uses of the word "indeterminate", and both are in the progressbar section.

@mcking65
Copy link
Contributor

mcking65 commented Jul 7, 2020

@jongund wrote:

Reading the Section 8.1 on range guidance there seems to be a disconnect for ranges that are indeterminate and using default values for range attributes. The guidance for indeterminate is to not include aria-valuemax or aria-valuemin, but we also say when one or both of these attributed are not included that the default values of 0 and 100 will be used. So which is it, default values or indeterminate? Maybe it should be when the aria-valuenow attribute is missing or empty it should be the way to author identifies an indeterminate state.

The only widget that allows for unknown range is spinbutton. Even a progressbar that is indeterminate must have known limits. I reworded section 8.1 in commit 1985db4 to clarify. I hope this addresses your concern.

aria-practices.html Outdated Show resolved Hide resolved
Copy link
Contributor

@carmacleod carmacleod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is ready to merge now!

Copy link
Contributor

@jongund jongund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good

@mcking65 mcking65 merged commit 7efa6e5 into master Jul 7, 2020
@mcking65 mcking65 deleted the zcorpan/range-related-properties-2 branch July 7, 2020 21:39
michael-n-cooper pushed a commit that referenced this pull request Jul 7, 2020
Add guidance section for range related properties (pull #1279)

Resolves issue #255 by adding a section
That describes how to use aria-valuemin, aria-valuemax, aria-valuenow, and aria-valuetext.

Co-authored-by: Valerie R Young <[email protected]>
Co-authored-by: Matt King <[email protected]>
Co-authored-by: Carolyn MacLeod <[email protected]>
Co-authored-by: JaEun Jemma Ku <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants