Easy-digital-downloads: Multiple instances of variable product form causes label target issue

Created on 10 Jul 2015  路  11Comments  路  Source: easydigitaldownloads/easy-digital-downloads

When there are multiple instances of the [purchase_link] shortcode on a page for the same variable product, clicking on the labels takes you to the first instance of the shortcode instead of just selecting the radio option as I suspect a user would expect.

For users with long sales pages and repeated calls to action throughout, this creates a potentially confusing experience.

scope-ui type-bug

Most helpful comment

@cklosowski All good still after pulling issue/3598 down again.

All 11 comments

@brashrebel there is a caveat to this without some extra JS involved. The issue is that, while we make the IDs on the forms themselves unique, we weren't making the inputs themselves, so the labels all had the same id attributes, by appending the form_id to the input ids we can make it so the labels work with the same form. They caveat is that picking a price ID on one form, will not update the other form without some JS.

@easydigitaldownloads/core-devs are we ok with that?

Why not just update the labels with the ID? That doesn't require any JS.

@pippinsplugins that's what I'm doing...however it won't update both forms when you select an item. You cannot have two elements with the same id, by doing so you break HTML compliance, and also, no matter what, clicking on a label on any non-first form will scroll you to the first form.

I'll commit what I have for testing.

Replicated and confirmed fix for me on issue/3598. Also tested adding price options to the cart from multiple purchase forms to make sure the right item was being added. All good.

@SDavisMedia can you test again? Had to fix a merge conflict and want to make sure I didn't break anythign after re-adding the schemea stuff that was merged.

@cklosowski All good still after pulling issue/3598 down again.

@brashrebel does this resolve the initial issue for you? If so i'll merge and close this out

Merged and closing. Will re-open if @brashrebel notices anything.

Confirmed fixed for me in release/2.6.

Thanks @brashrebel

Was this page helpful?
0 / 5 - 0 ratings