Easy-digital-downloads: <legend> output in forms

[justintadlock]

It is my understanding that a <fieldset> should only have one <legend> and that should be the first child element. Legends are currently wrapped in a <span>, so they're not the first child.

The fieldset and legend form a pair. The legend is the "caption" for a particular fieldset. You can have multiple <fieldset> elements, each with their <legend>, within a single <form> (e.g., would do this in the profile editor form).

I'm just bringing this up because how the <legend> element is done in the plugin messes up the design in every one of my themes.

I see two good options:

• Remove the <span> wrapped on <legend> elements and create extra <fieldset> elements in forms with multiple <legend> elements.
• Drop the <legend> element altogether and opt for an <h2> or something like that.

I tried to do a little research on this because I've never seen legends handled like this before. Here's some specs:

• http://www.w3.org/TR/html5/forms.html#the-legend-element
• http://www.w3.org/TR/html5/forms.html#the-fieldset-element
• https://developer.mozilla.org/en-US/docs/Web/HTML/Element/legend
• https://developer.mozilla.org/en-US/docs/Web/HTML/Element/fieldset

[pippinsplugins]

Thanks for the report @justintadlock!

Tagged for 2.4.3.

[pippinsplugins]

I vote we drop the span tags as that will have the least impact on existing styles.

[pippinsplugins]

@SDavisMedia could you give this a quick test in your themes to make sure no styling is broken?

[SDavisMedia]

It's definitely making a difference. Gonna look at some CSS now to see what it'll take to correct.

[pippinsplugins]

I didn't see any differences in Twenty Fifteen. Also confirmed that EDD core doesn't style off of the span tags.

[SDavisMedia]

I'm seeing Twenty Fifteen changes, the same changes I see in Vendd. I don't style off the span either. There's something going on here. What does the "Personal Info" legend text look like for you? Like this? http://glui.me/?i=85aki7ys6m7ktyl/2015-08-12_at_2.54_PM.png/ If so, compare it with master and let me know if you see a difference.

[pippinsplugins]

Maybe it's coming from theme styles.

I'll check again.

[pippinsplugins]

Punting to 2.5 since there are apparent style changes. Can't make style changes in a point release.

[SDavisMedia]

I'll study this again this weekend.

[pippinsplugins]

:+1:

[spencerfinnell]

Would definitely break Marketify.

[pippinsplugins]

@spencerfinnell so that we have more to go off of, could you show me how it breaks?

[spencerfinnell]

Just styles, but would definitely require an update.

http://www.screensnapr.com/qs4v0jn0xpnb

vs

http://www.screensnapr.com/3ur8ybbpxivz

That being said, the extra element definitely helps our styling. Switching the spans to the inside would help us, but would still require an update.

Depending on timing we could make any changes for Marketify 2.0.0. Is there a timeline for 2.6?

[pippinsplugins]

@spencerfinnell 2.6 is months out. Maybe Jan or Feb of 2016. If you could do an update before hand to help us make the transition, that'd probably be best.

[spencerfinnell]

Yep, I'll add some compatibility to 2.0.0 that assumes the outer spans will be removed eventually.

[cklosowski]

@SDavisMedia @pippinsplugins looks like @spencerfinnell got this done anticipated us making changes, can we get this going to so we can make the 'breaking' change in 2.6? I'd hate to put this off for another major version.

[pippinsplugins]

Let's get the pull request up to date (if not already) and then start testing our own themes. @SDavisMedia could you do a round of testing? We'll need to do this ASAP so we can announce it and give a heads up through the dev blog.

[SDavisMedia]

Got it.

[cklosowski]

@SDavisMedia I've updated the branch issue/3568 with the latest from release/2.6 so go ahead and test out.

[SDavisMedia]

okay will do

[topher1kenobe]

I see the spans on master, see them removed properly on issue/3568 and in Vendd they still look good.

[pippinsplugins]

We'll need to test with all of the major themes:

• Vendd
• Shoppette
• Marketify
• SquareCode
• Stocky
• Digital Store (discontinued but still used)

[topher1kenobe]

Rock on, away I go

[topher1kenobe]

In Shopette, without the span the legend moves up and straddles the div edge.

With span:

Without span:

[pippinsplugins]

@SDavisMedia Could you take care of that in Shoppette?

[topher1kenobe]

Marketify keeps its spans, even on issue branch 3568, and looks fine.

It looks like they moved them inside the legend though.

[topher1kenobe]

Digital Store moves the legend up to straddle the border when the spans are removed.

With:

Without:

Honestly I don't think it's a problem with this theme, I'd leave it.

[SDavisMedia]

@topher1kenobe make issues and assign me on the repos of the themes you have problems please.

[topher1kenobe]

SquareCode looks exactly the same with and without spans.

[topher1kenobe]

Stocky makes the legend move up to straddle the border when there's no span.

[pippinsplugins]

Excellent. Let's also go ahead and do a round of tests on the last three default WP themes please.

[justintadlock]

I just wanted to confirm that the changes have fixed my original issue posted. Nice work! Now, I need to get back to work on the EDD theme I'm building. :)

[pippinsplugins]

:)

[pippinsplugins]

There are still a few themes that need to be updated (that we know of) but I'm going to go ahead and merge this into release/2.6 to ensure it gets as much testing as possible.

[pippinsplugins]

Merged into release/2.6 but leaving open with Needs Testing for additional testing.

[chriscct7]

There should be a dev blog post about this

[mindctrl]

@chriscct7 https://easydigitaldownloads.com/development/2016/05/20/markup-changes-on-checkout-in-version-2-6/

[SDavisMedia]

I've made adjustments to Vendd, Shoppete, Lattice, and Digital Store. Quota already controlled these styles. All of the theme updates will go out before EDD 2.6. All changes are backwards compatible.

[pippinsplugins]

Stripe has to be deployed at the same time as this: https://github.com/easydigitaldownloads/edd-stripe/issues/108

[SDavisMedia]

I'm gonna go ahead and push an update to all of the themes. They will all be fine with both the old markup and the new markup.

[topher1kenobe]

2014 looks fine in my opinion.

[topher1kenobe]

2015 looks fine.

[topher1kenobe]

2016 is good.

[topher1kenobe]

@justintadlock started this ticket because he's working on a new EDD theme, I'm talking to him about testing that as well.

[topher1kenobe]

2013 is good.

[topher1kenobe]

2012 is good.

[topher1kenobe]

2011 is good.

[topher1kenobe]

2010 is good.

[topher1kenobe]

Basically it looks great on all themes that leave default styling. I'm going to call this one tested and done.

[SDavisMedia]

All EDD themes are updated.

[pippinsplugins]

I think we go ahead and close this :)