-
Notifications
You must be signed in to change notification settings - Fork 194
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
select: Should <selectedoption>
respond to mutations in the selected <option>
#825
Comments
i vote for the value to update to bar |
It should display whatever the form submitted value would be imo. So if that code would update the value (I think it would) then it should also update the displayed html |
There hasn't been any discussion on this issue for a while, so we're marking it as stale. If you choose to kick off the discussion again, we'll remove the 'stale' label. |
Yeah we should probably listen to mutations, it really seems like the sane default. It will likely be expensive for the browser to do that, but there are ways that we can improve the performance: https://chromium-review.googlesource.com/c/chromium/src/+/5370555/2/third_party/blink/renderer/core/html/forms/html_selected_option_element.h |
I propose that we resolve to make the selectedoption element respond to mutations inside option elements. Otherwise, we might not pick up updates that authors have made to the DOM and it will leave the selectedoption element in a broken state. |
The Open UI Community Group just discussed
The full IRC log of that discussion<brecht_dr> jarhar From the browser engineer perspective this is an interesting one<gregwhitworth> q+ <dandclark> q+ <bkardell_> q+ <masonf> q+ <brecht_dr> jarhar If you append some text in the option it would also show up in the button. is this something we should do. I think authors want to do that. <Luke> +1 <brecht_dr> jarhar I would think we want to support that <brecht_dr> gregwhitworth I agree we should. We did resolve on the inverse that the content is the innertext that is migrated. <brecht_dr> gregwhitworth we should point that out in our documentation <gregwhitworth> ack gregwhitworth <gregwhitworth> ack dandclark <brecht_dr> dandclark I agree with jarhar. <gregwhitworth> ack bkardell_ <brecht_dr> bkardell_ I was curious. What is the case for somebody to do this? <Luke> q+ <brecht_dr> jarhar Let's say you have some website where it has hydration kind of thing and your initial html you have a select element and then you have a seperate call in a database where you want to update your select options for example some imates <brecht_dr> jarhar and you want to update that button as well by updating (hydrating) the options <gregwhitworth> ack masonf <brecht_dr> masonf I do have a fear about specifying this behaviour. <gregwhitworth> q+ <keithamus> q+ <gregwhitworth> ack Luke <brecht_dr> masonf I'm still wondering about real use cases <brecht_dr> Luke How does a lazy loaded image cloned into the new area work? <brecht_dr> masonf That should be ok? as it's loaded by its src link <gregwhitworth> ack dbaron <brecht_dr> masonf this seems more like a rendering question instead of cloning question <brecht_dr> dbaron I find the hydration example to be convincing <scotto9> q+ <masonf> q+ <brecht_dr> gregwhitworth The root scenario in the issue is the most common use case that I would expect. I'd expect that to work <gregwhitworth> ack gregwhitworth <gregwhitworth> ack keithamus <scotto9> q- <brecht_dr> keithamus If I were make this as a web component, I would make the selected option a slot. and do manual slot assignment <dbaron> s/to be convincing/to be convincing, particularly for whatever is selected on initial page load/ <bkardell_> it does seem very similar <brecht_dr> keithamus I'm sure the implementation is different, but I wonder why we shouldn't do it, because I would do it like this as a web component <bkardell_> I think if you find the <mirror> proposal/ideas it used a similar argument <gregwhitworth> +1 to bkardell_ <brecht_dr> masonf You'd actually have to clone, it should be in both places <gregwhitworth> q? <brecht_dr> masonf It needs to be cloned as you might want to style it differntly <brecht_dr> gregwhitworth The same goes for accessibility reasons <gregwhitworth> q? <gregwhitworth> ack masonf <jarhar> Proposed resolution: The selectedoption element should update its contents when the selected options DOM descendants are modified <brecht_dr> una There is a gmail example where there is an icon int he button and additional text in the dropdown when opening <Luke> +1 <dandclark> +1 <keithamus> +1 <masonf> +1 <brecht_dr> +1 <jarhar> RESOLVED: The selectedoption element should update its contents when the selected options DOM descendants are modified <scotto9> caveat - if you are dynamically updating the selected option with a counter/countdown - please stop <scotto9> stop coding in general <scotto9> :) |
reopening due to whatwg/html#10520 |
Was the following alternative considered? If the content of the
<select>
<button>
<selectedoptions>
<selectedoption value="a">custom a</selectedoption>
</selectedoptions>
</button>
<option value="a">a</option>
<option value="b">b (this will be cloned when selected)</option>
</select> |
I think that for more sophisticated cases where you don't want to clone but instead want to have other content that lives in the option which gets rendered when a particular option is selected, this could be implemented by not using the We wanted to add this element to support basic cases so that most users of this new select would not have to use javascript to copy the innerHTML on every change event, but since its a separate element that you can choose to put in the button or not, we are providing flexibility to build more sophisticated things in userland. Based on the discussions so far, and my experimentation of implementing different things and testing out basic cases with React, I am supportive of not observing mutations inside select or option elements and only cloning during initialization (parsing) and when new options become selected (when the change event fires). This is the first option in jake's article. At first I thought that not observing mutations would not work in basic cases, but I have been convinced otherwise after implementing it and experimenting with React, as well as the idea of adding a resetContent method that authors can use if needed. |
<selectedoption>
respond to mutations in the selected <option>
<selectedoption>
respond to mutations in the selected <option>
The Open UI Community Group just discussed
The full IRC log of that discussion<hdv> JakeA: when you select an option, or the default one becomes selected, the content of the selected option is cloned and light dom inserted inside of selectedoption, which is optional<hdv> JakeA: my question is… while there's content in one of the options, waht should happen if you modify one of those options <hdv> JakeA: originally it would get recloned, but based on feedback from webkit, who said it was a bit weird… so next step was to do it synchronously <hdv> JakeA: problem with that was… a basic modification of the dom was going to mutate many many times <hdv> JakeA: that's going to happen a lot more than people expect <hdv> JakeA: i think the biggest problem with it is that there's no way to stop the clone or modification happening <hdv> JakeA: there will be cases where people make a change to the content of the option because they want to change the content of that option <hdv> JakeA: eg we don't want hover effects to be duplicated over <hdv> JakeA: what makes me worried about this is that we get more and more elements that modify their own attributes… am not thrilled about it, but it seems to be the new hotness (hidden-until-found, details/summary, I think dialog does it too) <hdv> JakeA: I know we can't put details inside option, but if we could it would expend in one place and also inside of the selected option <hdv> JakeA: so I wondered… what do we do… I asked for developer feedback <hdv> JakeA: what I heard the most… people who are using React or another framework are not going to use this <masonf> q? <brecht_dr> q+ <masonf> q+ <gregwhitworth> q+ <hdv> JakeA: one simple thing we could do…is to say… when you select the option, it clones then, andit will never reclone unless you do it manually… and we could potentially some easier way to do that <hdv> JakeA: we already have `replaceChildren()`…the bit we're missing is `cloneChildren()`, that would provide an easy way to do it… or a method on `<selectedoption>` <gregwhitworth> ack brecht_dr <hdv> brecht_dr: +1 on not changing the selectedoption more than once <hdv> brecht_dr: oh ja <hdv> s/oh ja// <gregwhitworth> q+ steveOrvell <gregwhitworth> ack masonf <hdv> masonf: my concern was React, but think most people in React would use a portal or something and do it themselves, but in React it would be easy anyway to do it <hdv> masonf: I had been thinking about a JS API on the select element that would sync the options <hdv> masonf: you brought up clonechildren or a method on the selectedoption element <gregwhitworth> q+ sarah <hdv> masonf: they sound fine to me too <gregwhitworth> ack gregwhitworth <gregwhitworth> q+ <hdv> masonf: there are two things that cause the clone… the user or when it is originally parsed <hdv> JakeA: the use case is, you've got an app that renders the custom select, then in the use effect it is going to get the real option and call setOptions <hdv> JakeA: in that case selectedoption would still display 'loading' as React is still cloning… to get that faulty case I actually had to play around with it a bit… and in some cases eslint would scream at me too <hdv> JakeA: @@@, I know Anne isn't a fan of this, but script element, style element already do this <gregwhitworth> ack steveOrvell <hdv> steveOrvell: today, the non custom select behaviour, if you modify selected option, it will synchronously update the selected option <hdv> JakeA: how is it observably synchronous? <hdv> steveOrvell: you can look at the offset width <hdv> JakeA: of course <hdv> steveOrvell: another point is re a11y: there are many ways to mess up the a11y of the custom select… the goal of Open UI is to not make things that make it easy to mess up accessibility. <hdv> steveOrvell: the thing that is announced is the actual selected option , so anything you do visually would cause a discrepancy between what's visual and what's the accessible name <hdv> JakeA: that's an important detail we need to make sure <scott> q+ <hdv> masonf: what you just said Steve is a bug, what's there visual should be reflected in a11y tree, I'll make sure that gets fixed <gregwhitworth> ack sarah <hdv> sarah: for a11y I would prefer it to not change automatically… <hdv> sarah: if you change the value that the keyboard focus is on, the 'screenreader cursor' would be pulled back to that. It treats the active descendant as @@@2 <hdv> sarah: but in this case focus seems to be actually on the button not the options, in which case it would not be a problem, as the screenreader user would get an equivalent experience <hdv> sarah: also, if somebody is mutating the contents of the option element… they're obviously using script for this, it is trivial to manage what's displayed without using selectedoption … so in this case the dev likely has some kind of render function and we don;'t worry about handling that as much in selectedoption <hdv> JakeA: seems reasonable <hdv> JakeA: if the dev has chosen to not use selectedoption but they choose to put something in, screenreader users should see that. But if they are using selectedoption … if they choose to show/hide parts of the tree, it's important that screenreader users see that <hdv> sarah: yes. Also screenreader users see one thing at the time <hdv> sarah: at each point it doesn't matter if the selectedoption and option content are equivaelnt <hdv> s/equivaelt/equivalent <hdv> JakeA: somebody asked on HackerNews if they wanted different content in the option and selectedoption… I answered you would probably do it manually <hdv> sarah: in that case it would work because display:none affects the acc tree <hdv> JakeA: but my understanding is in current implementation it is broken <hdv> masonf: currently is broken, we'll fix it <hdv> s/currently is/yes, it is currently <hdv> gregwhitworth: I'm confused on what the actual problem is when I select again? <hdv> masonf: no if the user did it it would clone twice <hdv> JakeA: this particular user wanted instead of cloning the option that there is a … <hdv> gregwhitworth: oh I didn't mean that user, I mean the general issue here <hdv> JakeA: if we clone the DOM whenever anything changes, we'll get a load of clones, if it is asynchronously, and authors might not want that reflection <hdv> masonf: we'll clone for the change 'user clicks', but the issue JakeA is talking about is when the author does some kind of modification via script <gregwhitworth> ack gregwhitworth <hdv> JakeA: it clones whenever the option that is selected changes <keithamus> q+ steve <gregwhitworth> ack scott <hdv> scott: some of what I was going to say was brought up <hdv> scott: I see people asking similar things re modifying options or changing the content of the selected option after the fact… <masonf> q+ <gregwhitworth> q+ <hdv> scott: people want to do complex stuff <hdv> scott: people could mock around in the past, as steve says, but with this custom select it is much easier to make this a broken experience <gregwhitworth> ack steve <hdv> scott: re the bug to file I'm not sure if there's a clear answer to what value should be returned or announced <hdv> sorvell: to go fruther down the rabbit hole… if you're modifying the DOM inside the `<selectedoption>` via script, we can say, that's bad, don't do it … is that confusing? (1) if we really didn't want it to happen, is there a way for us to say it cannot be modified? maybe we can tell people? (2) are we potentially limiting cool interactive patterns? <hdv> sorvell: the cloning behaviour means you should not mess with this stuff <hdv> JakeA: if you say the behaviour is it clones… it's quite simple, there's; an event when this changes <hdv> JakeA: you can do it via CSS so why not via the DOM <gregwhitworth> ack masonf <hdv> masonf: since we're defining the behaviour… it's fine if you modify the contents of selectedoption, but you know it will be blown away when someone changes the option. Am afraid of inventing some kind of unmodifyable thing <sarah> q+ <scott> q+ <hdv> masonf: current behaviour is to clone the children, we should probably also clone the attrs or maybe the whole option… we should probably think about that at least <JakeA> q+ <hdv> masonf: there's some kind of algo of what happens when it changes, the author should be able to access that exact algorithm or evnet <hdv> s/evnet/event <hdv> gregwhitworth: we brought up what to clone, eg styles and everything… so should we be cloning the attributes? maybe if you narrow it down to specific attributes, it would become somewhat weird DX. I think we said not to do attrs because the style attribute would cause weird edge cases <hdv> gregwhitworth: we're not changing how the tree is constructed, more naming… how are we breaking accessibility? <gregwhitworth> ack gregwhitworth <hdv> sarah: re cloning attributes… my immediate reaction is 'oh god no', the content of the thing is the value of the select not the name , if we clone an aria label of an option onto the button of the select, we've wiped out the name of the button, and then is someone uses aria-checked instead of aria-selected and that gets cloned… could be a very bad idea for anything that can affect semantics <keithamus> q+ steve <jarhar> proposed resolution: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <hdv> ack sarah <gregwhitworth> ack sarah <masonf> q+ <gregwhitworth> ack scott <hdv> scott: to allude to the codepen I've shared… what happens is that the aria label on the option is return as the value… which is not… there are going to be use cases where we'll absolutely want that <hdv> scott: should we reflect all the attributes on the selectedoption for instance, I'd say no… right now, anything you put on the button or selected option is going to be ignored, it's not where the values are computed from <hdv> scott: selectedoption… I'm really starting to think we should treat it like slot… I think we shouldn't compute the value from that <hdv> scott: because then the selectedoption could be modified in other ways <gregwhitworth> ack JakeA <hdv> JakeA: would putting a div with aria-label inside of the option, would that avoid the problem when cloned into selectedoption? <hdv> scott: this is another area we should align on what the expected behaviour should be… eg a div is not supposed to be named at all in the aria spec <hdv> scott: as long as the option returns its value which is the same as its name, arguably that should become a returned value <hdv> scott: if it matches the visible text that's up to the developer, sometimes you might not, in the currency picker that Una made there was some good reasons to purposefully pick what's the acc name, to avoid the £ sign getting read out twice <jarhar> proposed resolution: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <hdv> s/twice/many times <gregwhitworth> ack steve <hdv> sorvell: people say different things about accessibility… previously we talked about what's in the option should be announced the same… and others say it should not be the same <hdv> gregwhitworth: if somebody sets a thing to display:none in CSS… that affects acc tree too <hdv> gregwhitworth: when I select an option, focus goes back to the outer button, correct? <hdv> masonf: correct <hdv> keithamus: we're in unchartered territory, could go either way <jarhar> proposed resolution: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <keithamus> q+ <hdv> masonf: these are all great issues, we need to nail down what the a11y behaviour is, though I think that's not what we're resolving here <gregwhitworth> ack keithamus <masonf> q? <gregwhitworth> ack masonf <hdv> keithamus: to +1 what masonf's says… the concern in the issue is what is the DX, because there are timing issues, it doesn't really affect the observable thing for the user? <jarhar> proposed resolution: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <hdv> keithamus: re the original question: should the option's contents clone into the selectedoption on the activation of the option, that seems like a reasonable thing to me <hdv> sorvell: the ability to put the same DOM into multiple places … this could result in weird things we should account for, it gives a lot of responsibility to folks customising their selects <hdv> sorvell: you could put iframes in etc… this is an advanced use case and it can only go so far, but we want to try to make the default behaviour coudl <hdv> s/coudl/good <hdv> masonf: agreed with that… we're opening up great power here and that comes with great responsibility <jarhar> proposed resolution: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <keithamus> +1 <hdv> masonf: am on the queue to say: I would like to withdraw my proposal to @@@3 and still keep my proposal to add some kind of 'synchronise selected option' method <JakeA> +1 <hdv> +1 <sorvell> +1 <brecht_dr7> +1 <sarah> +1 <jarhar> RESOLVED: dont observe mutations in option elements. only clone into selectedoption during parsing and when a new option becomes selected <brecht_dr7> (on another name, irc did something strange, no with nr 7) <keithamus> q+ <hdv> masonf: is #825 a v2 thing? <gregwhitworth> ack keithamus <hdv> masonf: I would like a JS method that does exactly what the browser does when someone selects an option <hdv> keithamus: I agree with that <hdv> keithamus: and also… in multiple selects the options are an array, there's something you'll have to do there anyway <hdv> keithamus: probably something to resolve there <hdv> JakeA: like the proposal, what the method name is etc is up for grabs <hdv> sorvell: I wanted to zero in on one thing Jake said… when cloning occurs, we won't rely on an event? <hdv> JakeA: I think that was a misunderstanding of what gregwhitworth was saying… he said when there is an event when does it occur? <hdv> gregwhitworth: let's resolve on the spirit, then the spec text <hdv> JakeA: eg if you change checked attr on a checkbox you don't get an event either <hdv> JakeA: can't remember what the programmatic ways are to make changes… eg if you set selectedindex, that is 1, to 1, would that reclone or not, because you set it to the same? <hdv> sorvell: are we ok with the developer being unable to respond to that? <hdv> JakeA: you could put a mutationobserver on the selected option <hdv> sarah: are you saying… if you have one set of options rendered, when user clicks on the same index but it is the same option it would not fire a change event? <hdv> JakeA: I don't have an answer, would be something we'd need to decide <hdv> masonf: even if you don't mutate but the user clicks the same option that's already selected, feels to me it should still clone <hdv> jakea: selectedindex = selectedindex gives you that clone operation |
Since people sounded more interested in accessibility mappings of the button than the cloning behavior, I created a separate issue here so we can talk about it: #1116 |
#1119 follow up on when the cloning should happen in relation to selecting the selected option. |
This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b
This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573}
This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573}
This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573}
… mutations, a=testonly Automatic update from web-platform-tests Make <selectedoption> stop responding to mutations This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573} -- wpt-commits: ff425b2cbd0763d18844cf91cde9da9c8616f076 wpt-pr: 48913
… mutations, a=testonly Automatic update from web-platform-tests Make <selectedoption> stop responding to mutations This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573} -- wpt-commits: ff425b2cbd0763d18844cf91cde9da9c8616f076 wpt-pr: 48913
… mutations, a=testonly Automatic update from web-platform-tests Make <selectedoption> stop responding to mutations This patch makes <selectedoption> stop using the MutationObserver for <option>: openui/open-ui#825 This patch also uncovered a small bug with the selectedoptionelement attribute when assigning null to the IDL attribute, which I fixed by removing a null check. Bug: 336844298 Change-Id: I3fed171bf687cabb9474d8ff8741071c28bbad0b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5941575 Reviewed-by: Dominic Farolino <[email protected]> Commit-Queue: Joey Arhar <[email protected]> Cr-Commit-Position: refs/heads/main@{#1376573} -- wpt-commits: ff425b2cbd0763d18844cf91cde9da9c8616f076 wpt-pr: 48913
Consider the following:
Should the
<selectedoption>
's contents set to "bar", or should it stay as "foo"?The equivalent use case for
<select>
will change the button's text to "bar".@argyleink @mfreed7
We should probably wait for #571 to be resolved before getting this 100% figured out.
The text was updated successfully, but these errors were encountered: