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

Update: legend to provide name to optgroup #2360

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

scottaohara
Copy link
Member

In regards to the updated content model for the select element and its allowed children, an optgroup can have a legend element as its first child, and this legend needs to be able to name the optgroup similarly to how a legend names a fieldset.

see:
whatwg/html#10586

Test, Documentation and Implementation tracking

Once this PR has been reviewed and has consensus from the working group, tests should be written and issues should be opened on browsers. Add N/A and check when not applicable.

  • "author MUST" tests:
  • "user agent MUST" tests:
  • Browser implementations (link to issue or commit):
    • WebKit:
    • Gecko:
    • Blink:
  • Does this need AT implementations?
  • Related APG Issue/PR:
  • MDN Issue/PR:

In regards to the updated content model for the select element and its allowed children, an `optgroup` can have a `legend` element as its first child, and this `legend` needs to be able to name the `optgroup` similarly to how a `legend` names a `fieldset`.

see:
whatwg/html#10586
Copy link

netlify bot commented Oct 22, 2024

Deploy Preview for wai-aria ready!

Name Link
🔨 Latest commit e543559
🔍 Latest deploy log https://app.netlify.com/sites/wai-aria/deploys/6725291214eaf90008245668
😎 Deploy Preview https://deploy-preview-2360--wai-aria.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

open question in whatwg/html#10586 (comment) about what should take priority - legend or optgroup's label attr.

if both end up being allowed / render - then one of these could be added to the accDescription computation - instead of ignoring one or combining them into one long name.
@scottaohara scottaohara marked this pull request as draft October 24, 2024 17:19
Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

Couple of minor formatting comments, but this LGTM from a spec position, modulo new resolutions from the WHATWG.

html-aam/index.html Outdated Show resolved Hide resolved
html-aam/index.html Outdated Show resolved Hide resolved
Co-authored-by: Keith Cirkel <keithamus@users.noreply.github.com>
@scottaohara scottaohara marked this pull request as ready for review November 1, 2024 19:02
@scottaohara
Copy link
Member Author

@keithamus would you mind taking another look? I've updated the ordering for the optgroup naming, and added new content to how a description should be calculated.

cc @josepharhar as updates here related to my comments in the update content model for customized select PR

@scottaohara
Copy link
Member Author

examples for tests for how name/desc steps should work


<!-- name of optgroup: foo -->
<select>
    
    <optgroup aria-label=foo label=bar>
        <option>test
    </optgroup>
</select>

<!-- name of optgroup: foo 
        desc of optgroup: bar
-->
<select>
    
    <optgroup aria-label=foo title=bar>
        <option>test
    </optgroup>
</select>


<!-- name of optgroup: foo 
        desc of optgroup: bar
-->
<select>
    
    <optgroup label=foo title=bar>
        <option>test
    </optgroup>
</select>

<!-- name of optgroup: bar -->
<select>
    
    <optgroup title=bar>
        <option>test
    </optgroup>
</select>

<!-- name of optgroup: bar -->
<select>
    
    <optgroup label=bar>
        <option>test
    </optgroup>
</select>

<!-- name of optgroup: foo 
        desc of optgroup: bar
-->
<select>
    
    <optgroup aria-labelledby=x label=bar>
        <legend id=x>foo</legend>
        <option>test
    </optgroup>
</select>


<!-- name of optgroup: bar 
        desc of optgroup: foo
-->
<select>
    
    <optgroup label=bar>
        <legend>foo</legend>
        <option>test
    </optgroup>
</select>


<!-- name of optgroup: foo 
     desc of optgroup: bar baz
-->
<select>
    
    <optgroup aria-label=foo label=bar>
        <legend>baz</legend>
        <option>test
    </optgroup>
</select>


<!-- name of optgroup: foo 
     desc of optgroup: bar baz
-->
<select>
    
    <optgroup aria-label=foo label=bar title=ignored>
        <legend>baz</legend>
        <option>test
    </optgroup>
</select>

Copy link
Member

@keithamus keithamus left a comment

Choose a reason for hiding this comment

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

Still LGTM

@scottaohara scottaohara marked this pull request as draft November 12, 2024 22:23
@scottaohara
Copy link
Member Author

scottaohara commented Nov 12, 2024

put back into the draft state as I need to revise the bits about how a label attribute and legend element should no longer be expected to render at the same time. re: https://issues.chromium.org/issues/378601807

@josepharhar
Copy link

put back into the draft state as I need to revise the bits about how a label attribute and legend element should no longer be expected to render at the same time. re: https://issues.chromium.org/issues/378601807

If you have a recommendation for what to do in this case, let me know and/or comment on the crbug. I haven't implemented anything yet to account for the case where both are present.

@scottaohara
Copy link
Member Author

@josepharhar i think it's totally fine to only have one render / and have a preference for the legend element in this case since it'll allow for authors to do more.

  • if only label attribute is specified, render that to be consistent with prior select functionality
  • if only legend is specified, render that and if the select is loaded in a browser that doesn't support customization, there is no fallback
  • if both are specified,
    • in browsers that support customization render legend element as the group label and do not render label attribute text
    • in browsers that don't support customization render the fallback label attribute text as the group label

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

Successfully merging this pull request may close these issues.

4 participants