Icons in the frontend and performance

[samikeijonen]

There are couple of icons in the frontend which are used as custom font icon or dashicons. We could probably move away from icon fonts and use SVGs only.

• First example is from checkout page. There we load all dashicons if SSL is around just for padlock icon. We could use SVG icon instead.
• Second example is the Ok icon before the Added to chart text. We could use SVG for that.
• Third example is the spinner. We could probably do that with CSS only.

I'm not sure are the icons edd-icon-remove, edd-icon-remove-circle, and edd-icon-ok-circle used anywhere.

I can create PR for this. I do understand that this might have backwards compatibility problems with themes. Another solution is to add some hooks where we can insert icons without JS.

[pippinsplugins]

I like this idea.

I doubt there'd be much BC concern here.

[samikeijonen]

Ok. I create PR for this for testing in couple of days.

[pippinsplugins]

Since this does have potential backwards compatibility ramifications, I'm going to milestone it for 2.7.

[samikeijonen]

Anybody had time to test this?

[pippinsplugins]

I have not yet but will soon.

[SeanTOSCD]

I was able to test this a bit. The spinner is super slick and the Added to Cart check is working just fine for me. Couple points:

1. I'm not set up to check the SSL lock so someone else needs to grab that please.

2. How about the loading.gif usage for the gateway picker switching and discount form submission loading (and probably other places)? I know that's not an icon font. But are we looking to adjust those too or were they intentionally left out?

[samikeijonen]

2. I just probably didn't notice them. I'll check if we can tackle them also.

…

On Nov 28, 2016 11:49 PM, "Sean Davis" ***@***.***> wrote: I was able to test this a bit. The spinner is super slick and the Added to Cart check is working just fine for me. Couple points: 1. I'm not set up to check the SSL lock so someone else needs to grab that please. 2. How about the loading.gif usage for the gateway picker switching and discount form submission loading (and probably other places)? I know that's not an icon font. But are we looking to adjust those too or were they intentionally left out? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#5166 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvG_3zPlB0c5k4lG_ZXA-62vibs1Y2Eks5rC0yAgaJpZM4KrT_D> .

[samikeijonen]

@SDavisMedia: In commit 9e23214 there is first pass of replacing loading.gif with the same spinner.

I'm not perfectly happy with small up/down movement in discount code but run out of time for today.

[pippinsplugins]

This is looking great!

The padlock on the CC form is definitely not working though.

Master:

Issue branch:

[samikeijonen]

I try to look into padlock icon. I just need to set up https for testing.

[SeanTOSCD]

@samikeijonen Sorry... forgot to mention I took a look at this during WordCamp US. The replacement for loading.gif looks great but I do agree that the slight up/down movement in the discount area does need to be hit. Could you squeeze that in there too?

[samikeijonen]

but I do agree that the slight up/down movement in the discount area does need to be hit.

Agree, I don't like it:) I was actually thinking does the loading spin have to be in the same spot as it use to be? Which is next to title.

Could it be be for example below Apply button? This was just an idea on top of my head at the moment. Not really tested anything.

[SeanTOSCD]

@samikeijonen that was the exact thought I had when I saw it too. I'd rather see it to the right of the Apply button, otherwise we'll still get a page jump, even if it's below the discount form.

[samikeijonen]

Sorry, I've been busy.

I'd rather see it to the right of the Apply button

Agreed. But I think we can do this only if we change the markup as <button>Apply (spinner)</button>. Would that be a problem for backwards compatibility?

[SeanTOSCD]

@samikeijonen yea, definitely don't want it inside the button. I was thinking to the right of it but that's probably a bad idea.

Instead, maybe you could try leaving it where it is but absolute positioning it so it doesn't affect the wrapper elements and cause the jerk? The Discount label is already relative positioned so that might work.

[samikeijonen]

I ment that I can have it on the right when switching to button markup. Then we can use position absolute on the spinner.

…

On Dec 17, 2016 1:28 AM, "Sean Davis" ***@***.***> wrote: @samikeijonen <https://github.com/samikeijonen> yea, definitely don't want it inside the button. I was thinking to the right of it but that's probably a bad idea. Instead, maybe you could try leaving it where it is but absolute positioning it so it doesn't affect the wrapper elements and cause the jerk? The Discount label is already relative positioned so that might work. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#5166 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABvG_x4NqyUFk0IIhLxNAM17mJK-xffMks5rIx6LgaJpZM4KrT_D> .

[samikeijonen]

Do we still have time to get this in 2.7?

Sorry I've been too busy to finish this up.

[samikeijonen]

I updated the PR #5177.

1. Padlock icon is working as inline SVG. EDD Stripe was just overwriting it. There is separate PR in https://github.com/easydigitaldownloads/edd-stripe/pull/175.
2. Apply discount spinner is tricky one. There is no golden CSS that fits for all themes. At the moment spinner is next to Apply button, or below it, depending on theme styles.

[SeanTOSCD]

I just checked out (all pun intended) the discounts spinner using Vendd. It's below the button. Honestly, I don't think it's bad. But it definitely disrupts the page and didn't do that before. This is a tricky one. Is there any way it can be smaller? Are there other options for loading icons? If it's smaller, it can stay where it was originally that way it doesn't make the page jerk.

[samikeijonen]

For Vendd I'd modify the theme CSS like this:

#edd_checkout_form_wrap .edd-cart-adjustment #edd-discount {
   display: block;
}

#edd_checkout_form_wrap #edd-discount-code-wrap input.edd-apply-discount {
   display: inline-block;
}

.edd-discount-loader {
   margin-top: 5px;
}

I'm out of ideas how all the changes would not affect in some way for themes out there. But in most cases there should be only small CSS tweaks.

[SeanTOSCD]

I really don't think it's bad below the button. The page is going to jerk anyway as the discount is added to the cart. I'm pretty happy with it.

let's see what @sunnyratilal thinks since he's doing a lot of work on 2.7.

[samikeijonen]

And like I said any theme can have the spinner on the right with some CSS.

[cklosowski]

@sunnyratilal @SDavisMedia assigning you two, just as a matter of organization as I work through 2.7.

[sunnyratilal]

I'm happy with the implementation.

[sunnyratilal]

Merged into release/2.7

[sunnyratilal]

Unit tests are breaking