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

Role spinbutton: allow empty values, no min, no max, and structure with sibling steppers #813

Merged
merged 1 commit into from
Oct 5, 2018

Conversation

mcking65
Copy link
Contributor

@mcking65 mcking65 commented Aug 31, 2018

For issue #797, allow empty spin buttons by:

  1. Changing aria-valuenow to supported from required property.
  2. Changing prose to author SHOULD from author MUST specify value for aria-valuenow.
  3. Adding aria-valuetext as supported property.

For issue #642, allow min and max to be undefined:

  1. Change aria-valuemin and aria-valuemax from required to supported properties.
  2. Revise prose of paragraph with normative statements.

For issue #812:

  1. Change description to clearly state that a text field with sibling buttons outside the spinbutton is permitted.
  2. Related editorial change to consolidate keyboard requirements into a single paragraph.
  3. Other related editorial revisions for clarity.

Preview | Diff

…th sibling steppers

For issue #797, allow empty spin buttons by:
1. Changing aria-valuenow to supported from required property.
2. Changing prose to author SHOULD from author MUST specify value for aria-valuenow.
3. Adding aria-valuetext as supported property.

For issue #642, allow min and max to be undefined:
1. Change aria-valuemin and aria-valuemax from required to supported properties.
2. Revise prose of paragraph with normative statements.

For issue #812:
1. Change description to clearly state that a text field with sibling buttons outside the spinbutton is permitted.
2. Related editorial change to consolidate keyboard requirements into a single paragraph.
3. Other related editorial revisions for clarity.
@jnurthen jnurthen merged commit 55db417 into master Oct 5, 2018
@joanmarie
Copy link
Contributor

joanmarie commented Oct 24, 2018

For future reference, let's aim for one pull request per issue.

From my initial triage of what we can already commit to the stable (Working Draft) branch, the issue #812 stuff doesn't strike me as something for which we need Core-AAM changes or tests; it's purely authoring.

On the other hand, this same pull request changed the default value of aria-valuenow from 0 to "that there is no current value" which should be tested. There may also be the need for a corresponding change in the Core AAM. Adding aria-valuetext as a supported property shouldn't need a change in the Core-AAM, but will need testing.

Do we block merging this entire set of changes into stable because of two single-line (but significant) changes? Do we manually break apart the changes and commit parts, but leave parts out? Neither is good imho. Solution: Please don't do this in the future. :) :)

joanmarie pushed a commit that referenced this pull request Oct 24, 2018
…th sibling steppers (#813)

* Change prose to author SHOULD from author MUST specify value for
  aria-valuenow.
* Change aria-valuemin and aria-valuemax from required to supported
  properties.
* Revise prose of paragraph with normative statements.
* Change description to clearly state that a text field with sibling
  buttons outside the spinbutton is permitted.
* Make related editorial change to consolidate keyboard requirements
  into a single paragraph.
* Make other related editorial revisions for clarity.
joanmarie added a commit that referenced this pull request Oct 24, 2018
Most of the changes made for pull request #813 were editorial or
were specific to authoring and thus did not require any changes
to Core AAM or testing related to exit criteria. Two of the changes,
however did not fall into this category namely:

* Make aria-valuetext a supported property of spinbutton
* Change the default value of aria-valuenow from 0 to "there is no
  current value"

Once we have ensured mapping changes have been made, tests are in
place, and implementations present or planned, we can merge the above
into the stable branch.
joanmarie added a commit that referenced this pull request Oct 24, 2018
Most of the changes made for pull request #813 were editorial or
were specific to authoring and thus did not require any changes
to Core AAM or testing related to exit criteria. Two of the changes,
however did not fall into this category namely:

* Make aria-valuetext a supported property of spinbutton
* Change the default value of aria-valuenow from 0 to "there is no
  current value"

Once we have ensured mapping changes have been made, tests are in
place, and implementations present or planned, we can merge the above
into the stable branch.
joanmarie added a commit that referenced this pull request Oct 15, 2019
The spinbutton role already supported aria-valuetext as a consequence of
subclassing the abstract role range. But it was added as an explicitly-
supported property of spinbutton as a consequence of #813. This part of
the change hadn't been merged due to the mistaken conclusion that new
tests would be needed.
@jnurthen jnurthen deleted the issue797-spinbutton-properties branch July 23, 2020 22:26
jnurthen added a commit that referenced this pull request Sep 8, 2020
@pkra pkra added this to the ARIA 1.3 milestone Jan 10, 2022
@pkra pkra mentioned this pull request Jan 10, 2022
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.

4 participants