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

Should we worry about existing button behavior? #809

Closed
bkardell opened this issue Aug 21, 2023 · 7 comments
Closed

Should we worry about existing button behavior? #809

bkardell opened this issue Aug 21, 2023 · 7 comments
Labels
select These are issues that relate to the select component

Comments

@bkardell
Copy link
Collaborator

bkardell commented Aug 21, 2023

Opening this out of an abundance of caution, but I noticed only in the past few days that 3 weeks ago as part of #702 it was suggested that we use a <button type="_something_"> and I just want to make sure that we're ok with that specific detail.

I think the idea here would allow an author to supplant this and if we do use that approach, it seems to me that in any unsupported browser it would result in someone clicking a button to, say, expand the list, and instead it submits the form.

Maybe everyone is ok with that on purpose somehow, but I'd really like to just make sure it didn't get overlooked (I overlooked it) because there is a lot happening in that issue. It seems that this detail originated as one of many detail during the call by josepharhar who then did a lot of real work to come up with an actual proposal for all of the things being discussed there.

Everyone had positive responses, including @domenic (who opened the issue with concerns from the HTML POV), so that's good - but re-reading it I'm not entirely confident that wasn't mostly about the nice shift in all of the other moving parts (which I also like).

@bkardell bkardell changed the title Should we worry about legacy button behavior? Should we worry about existing button behavior? Aug 21, 2023
@josepharhar josepharhar added the select These are issues that relate to the select component label Aug 21, 2023
@hidde
Copy link
Contributor

hidde commented Aug 22, 2023

Hm, now that you point it out… it does seem worrisome that authors could not progressively enhance the type="_something_" button, as the not-submitting-a-form bit would depend on the browser fully/properly supporting selectlist (and undoing that as part of their support).

@gregwhitworth gregwhitworth added the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Aug 22, 2023
@mfreed7
Copy link
Collaborator

mfreed7 commented Aug 24, 2023

Perhaps an alternative is to use a new attribute instead, and require type=button to avoid form submissions on non-supported browsers?

<selectlist>
  <button type="button" selectlistbutton>Pick a value</button>
</selectlist>

That new attribute would also have to be added to the HTMLInputElement:

<selectlist>
  <input type="button" value="Pick a value" selectlistbutton>
</selectlist>

But that'd do the trick, right? The name needs bikeshedding, of course. Perhaps activationbutton or something semi-generic?

The other approach, which I'd really like to avoid, is inventing a new element for this button:

<selectlist>
  <selectlistbutton> Pick a value </selectlistbutton>
</selectlist>

That's terrible by itself, because now you have yet another platform thing that is a button, but also, each new form control that we tackle will likely need its own set of new buttons. Think <datepickertodaybutton> and <fileinputpickafilebutton>, etc.

@josepharhar
Copy link
Collaborator

it was suggested that we use a <button type="_something_"> and I just want to make sure that we're ok with that specific detail.

I think that reusing <button> will just make things easier, but if there's anything that <button> can't do I'm all ears

I think the idea here would allow an author to supplant this and if we do use that approach, it seems to me that in any unsupported browser it would result in someone clicking a button to, say, expand the list, and instead it submits the form.

In an unsupported browser, <selectlist> will already act weird anyway right? I feel like the form submission of a type=selectlist button isn't a big deal in comparison. If I wanted to use selectlist and support an unsupported browser, then I'd probably not use selectlist until its supported or not use selectlist when running in that browser or use a polyfill that would take care of this issue.

@domenic
Copy link
Contributor

domenic commented Aug 24, 2023

I agree with Joey's position. If you're running any sort of script, e.g. a polyfill, it's easy to set up a preventDefault() handler in that script to prevent incorrect behavior in downlevel browsers.

@lukewarlow
Copy link
Collaborator

Could unknown elements that aren't valid custom element (single word name) be changed to disable default behaviour of buttons submitting forms? Probably not web compatible but just an idea.

@css-meeting-bot
Copy link

The Open UI Community Group just discussed [selectlist] Should we worry about existing button behavior? #809, and agreed to the following:

  • RESOLVED: Keep the current <button type=selectlist> proposal the way it is because polyfills would be required to make selectlist work anyway
The full IRC log of that discussion <hdv> topic: [selectlist] Should we worry about existing button behavior? #809
<hdv> github: https://github.com//issues/809
<hdv> masonf: the issue brought up here is about the danger that browsers that don't support selectlist would also not support the button behaviour, which would then make that those buttons trigger form submission as is the default for buttons in forms
<hdv> masonf: I agree it's an issue and we should think about it
<hdv> masonf: two options: one is to create a new element instead of an attribute… I don't really like it: at best confusing, at worst… terrible
<jarhar> q+
<hdv> masonf: another is don't use the type attribute but a different attribute, that we could bikeshed
<hdv> q+
<masonf> ack jarhar
<hdv> jarhar: I posted a comment on the issue… I don't think form submission behaviour is something to worry about… if you're in a browser that doesn't support selectlist, having selectlist wouldn't really make sense anyway
<hdv> jarhar: I don't think it's worth trying to address it… in those browsers you would probably do something completely different anyway
<hdv> masonf: on the other hand it's worse if it does something else and renders the form
<masonf> ack hdv
<scotto_> q+
<dbaron> hdv: Is the idea that developers would use the type attribute in addition to the other attribute?
<dbaron> masonf: yes, they'd need to use type=button along with the new attribute
<hdv> lukew: one way you could do this is @@@
<masonf> ack scotto_
<hdv> masonf: interesting suggestion, but could cause the opposite problem
<hdv> scotto_: that could also break custom elements as they are all unknown elements
<dbaron> s/@@@/make <button> elements inside of unknown elements not submit forms/
<hdv> scotto_: instead of using type= or another attribute… we would still run in the same issue if some developer _doesn't_ add the type attribute, if they forget it
<hdv> scotto_: as jarhar mentioned before… if someone uses this without a polyfill, many more things would break… a polyfill should address this case
<hdv> scotto_: I don't know how someone would use this without a polyfill
<masonf> q?
<hdv> masonf: the more I think about it, the more I agree… you could get lots of other funny business when you try to do this
<hdv> masonf: sounds like we're heading to the conclusion that this issue is moot
<hdv> masonf: any objections to resolving that
<hdv> +1 that makes sense
<dbaron> +1
<jarhar> Proposed resolution: Keep the current <button type=selectlist> proposal the way it is because polyfills would be required to make selectlist work anyway
<scotto_> +1
<jarhar> RESOLVED: Keep the current <button type=selectlist> proposal the way it is because polyfills would be required to make selectlist work anyway
<hdv> github: Naming of the selected value element

@gregwhitworth gregwhitworth removed the agenda+ Use this label if you'd like the topic to be added to the meeting agenda label Aug 26, 2023
@josepharhar
Copy link
Collaborator

I created another issue which poses similar questions to this one: #821

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
select These are issues that relate to the select component
Projects
None yet
Development

No branches or pull requests

8 participants