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

[Compatibility] Figure out the form attribute for IE11 #66

Closed
tyleralsbury opened this issue May 12, 2021 · 5 comments · Fixed by #394
Closed

[Compatibility] Figure out the form attribute for IE11 #66

tyleralsbury opened this issue May 12, 2021 · 5 comments · Fixed by #394
Assignees
Labels
Category: Bug Something isn't working Effort: Medium Severity: 1 Urgent Severity

Comments

@tyleralsbury
Copy link
Contributor

Purpose

We are using the form attribute on <input> elements in a few spots to facilitate sections everywhere - the cart and the product page so far, and I'm sure its use will be broadened over time as we continue to discover new use cases with Sections Everywhere.

The issue is that it is not supported on IE11, which still needs basic functionality like adding to cart and checkout. caniuse gives the rundown.

I don't think this should stop us from using it, but it might mean we need a polyfill which we had, in principle, decided were evil and not to be used.

@tyleralsbury tyleralsbury self-assigned this May 12, 2021
@tyleralsbury tyleralsbury removed their assignment May 31, 2021
@tauthomas01
Copy link
Contributor

One thing to note is that this form attribute is causing some issues to third party apps, including shop installments, in which the product form doesn't receive change event because the variant dropdowns are not wrapped in the main <form action="/cart/add"> element. In my reply, I made a test and put the <form> as the main wrapper of every form elements and everything works fine.

I'm curious what are the ups and downs of putting the <form> as the main container that wraps all form elements?

@tauthomas01
Copy link
Contributor

cc. @LucasLacerdaUX

@LucasLacerdaUX
Copy link
Contributor

I'm curious what are the ups and downs of putting the

as the main container that wraps all form elements?

The main issue is that, if we wrap all the blocks types, we'll likely end with <form> (or a lot of unwanted element types) inside <form> because we have:

  • Collapsible Tabs - which can use external page as source of content. And that page may contain a <form>
  • Popup, also uses a page as source of content.
  • App Blocks

So we decided to use this form attribute to link all the required fields to the product form, and creating multiple forms where needed (like product-installments being a separate form).

As the form blocks can be reordered freely, we can't even choose to have two {% for block in section.blocks %} loops in the page. So it looks like our only alternatives are wrapping EVERYTHING inside a <form>, which is an issue because of the things mentioned above, or using the form parameter :(

@tyleralsbury
Copy link
Contributor Author

tyleralsbury commented Jun 22, 2021

What we could do, though, is dispatch a change event on the form when the variant is changed. If that's what the installments app is listening for.

@nicklepine nicklepine transferred this issue from another repository Jun 28, 2021
@nicklepine nicklepine added Category: Bug Something isn't working Severity: 1 Urgent Severity labels Jun 28, 2021
@tyleralsbury tyleralsbury self-assigned this Aug 2, 2021
@tyleralsbury
Copy link
Contributor Author

For the scope of this issue, I've assigned myself to take a look at it. I'm leaning towards using a polyfill to make it work for IE11, even though I really don't want to use a polyfill. I can't think of anything better. 🤔

If the polyfill works outright, it should be pretty low effort.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Bug Something isn't working Effort: Medium Severity: 1 Urgent Severity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants