Problem/Motivation:
In updating the new version of the module, the form elements were changed and the selectors in the existing css files no longer apply.

Proposed resolution:
The css needs to be updated to reflect the new form elements, and any extra css removed.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aburke626 created an issue. See original summary.

JurriaanRoelofs’s picture

Thanks again for the upgrade, I'm looking at a diff between 1.x->2.x and I see a lot of time and energy went into this project. Also great that coding standard are highly respected and updated in lots of places! great work :)
I would love to help with this and I think I can handle the styling and CSS, but I wonder if we first need to change the form elements and try to get AJAX validation on coupon codes and VAT numbers working. Is this something RecurlyJS does for you and is it documented? or are we on our own here?

aburke626’s picture

I believe Recurlyjs only includes payment validation - I'm not sure if the PHP library may have more. Here's the documentation links for both:
https://docs.recurly.com/js/
https://dev.recurly.com/page/php

My guess is we are on our own.

JurriaanRoelofs’s picture

It turns out recurlyJS will validate everything automatically as long as it can interpret the form. it will validate some things via AJAX and some things on submission, you never really know what it's going to do

MurthyVittala’s picture

Assigned: Unassigned » MurthyVittala

Looking into it

MurthyVittala’s picture

Status: Active » Needs review
FileSize
53.1 KB
  1. The UI seems to be good with the base theme in Bartik
  2. Validations are happening on front end i.e from jquery side, for example the credit card validations
  3. There are also validations happening from the server side or Recurly JS for example the coupon codes

I assume this is fixed in between. Please attach the screenshots which would be useful if there is something missing.

Attaching the PDF for reference on how this looks in the Bartik base theme as of today.

markdorison’s picture

Status: Needs review » Needs work
markdorison’s picture

Assigned: MurthyVittala » Unassigned
Issue tags: -CSS

Moving this to unassigned due to lack of activity.

markdorison’s picture

Title: Refactor existing CSS for 7.x-2.x » Refactor existing CSS for 7.x-3.x
Version: 7.x-2.x-dev » 7.x-3.x-dev
Issue tags: +needs backport to 7.x-2.x
markdorison’s picture

eojthebrave’s picture

This probably also relates to #2683789: Update RecurlyJS library to v4 where all the CSS that was previously used to style elements on Recurly.js pages was removed. So now there's nothing in the module to refactor. And instead we need to add it back. Probably after going through what was previously removed and figuring out how to make it apply now.

As it is the Recurly.js form is pretty hard to use out of the box.

eojthebrave’s picture

Right now, the form elements show up, and work. Though they are relatively un-styled.

Couple of screenshots with Bartik for comparison.

Before:

Current:

The current form is missing some styling, and a bit of the additional information like for example the price, and term. I think it's probably best if we try and at least make the current UI match what it used to be. And we can then improve on the UI from there if desired.

eojthebrave’s picture

Assigned: Unassigned » eojthebrave
eojthebrave’s picture

Assigned: eojthebrave » Unassigned

I'm still tentatively working on this. But I'm also stuck getting the 7.x-3.x version of the module to work at all. So un-assigning this for now so as to not set false expectations of completing this anytime soon. :/

eojthebrave’s picture

I've put a bunch of work into getting this working on the Drupalize.Me website. Though, at the moment I don't have the time to break it all up into smaller patches. In the process of refactoring the code for the Recurly JS checkout page I've had to address these things:

Displaying the currently selected plan, it's price, any applied coupon codes, and a new order total after calculation. This is all handled a bit differently now with the V4 API. Mostly, you add data attributes to elements on the page and the Recurly JS library will locate and update those elements as needed. This patch adds a line that displays the currently selected plan and it's price. If the user enters a coupon code it also displays the discount and new price.

Coupon code validation. The old version allowed you to enter a coupon code, and then press a button to validate it. This now happens a bit more automatically, no button press required. But I added some code to detect both valid and invalid coupons and to display an error on the page if the coupon entered is invalid.

I refactored all of the CSS to work with the new form, which has a lot of different element names, etc. I added indicators to the CC# field for which type of card is being used, and generally tried to style things a little bit more. The style is different than the old form. Partially because the new API uses iFrames, and partially to bring it inline with the example code from the Recurly JS team.

I improved the error handling on the form so that users get a better indicator of which fields have an error, and friendlier error messages.

I added some code that ensures the first name, and last name entered into the billing form are used as the first name, and last name for the account that is created in Recurly when a new subscription is created. This brings the behavior back from the 7.x-1.x version of the module where the older version of the Recurly JS API did this for you automatically.

This also addresses, #2831806: Provide bare-bones template for the 'subscribe' form, and #2844377: Allow other module's to alter Recurly Account object before saving.

I realize this is a huge patch, and if I've got the time to do so in the future I'll try and refactor things into smaller chunks. But, but I wanted to at least share the code so that others could use it as a starting point if I don't get around to it soon.

eojthebrave’s picture

The patch above also helps address #2626662: Add recurlyjs form validation and markup.

And is sort of a duplicate of #2626660: Missing line items on RecurlyJS form which addresses many of the same issues.

I realize that this is a large patch, and might be somewhat unwieldy to try and review. And, I'm willing to try and help break it up a bit. However, it would be nice to at least get a sense of wether this is generally the right direction. Let me know if there is any other info I can provide.

walangitan’s picture

Excellent work across the board on this patch in #15!

I agree that splitting this patch out into several patches corresponding to their issues would be helpful for others to review. I can take the lead with creating separate patches and attributing them to @eojthebrave.

In terms of organizing each of these patches, here's how I'd organize portions of this patch:

#2831806: Visual appearance of the subscribe form -- code related to this ticket should stay in this one. I would lobby that the refactoring here addresses the visual defaults for subscribe form. It makes sense to keep them bundled here, I can link this issue on that one.

#2844377 : Allow other module's to alter Recurly Account object before saving -- can be it's own patch. I can take the lead here, post a patch on that issue and attribute it to @eojthebrave.

#2626662 : Add recurlyjs form validation -- could also be separated out into it's own separate patch.

Some of the work done here relating to coupons aren't currently in the queue. Should we keep them here instead of creating a new ticket/patch? I think that we should document the issue and include it here.

In terms of consolidating the patch for this issue, refactoring the CSS, minus the generic_card.png as I haven't been able to get that binary to apply. Is it possible @eojthebrave to post that image on this thread and I can attempt to re-roll the patch? After that, we can have a final review of this patch.

Overall, this is an excellent patch that restores and improves the experience for users installing/upgrading to the 7.x-3.x branch, well done @eojthebrave.

eojthebrave’s picture

FileSize
933 bytes

Here's the generic_card.png file.

And thanks @walangitan for taking the time to review, and help refactor this into appropriate chunks. I've been in a time-crunch to get something working on our site and didn't have time to juggle a bunch of different patches. I really appreciate it. I'll also try and keep my eye on this and other issues to help out where I can.

JurriaanRoelofs’s picture

FileSize
123.94 KB

About a year ago I posted a zip with my completed port of 7.x-3.x but I think it was never properly worked into the contrib module. I havent checked back to see progress here much and unfortunately I don't have free time to backport all updates neatly to here but in case someone has I'm attaching my working version of the 3.x branch. It was developed/finished by an external agency. It has been processing subscriptions on sooperthemes.com for about a year, supporting both cards and paypal. (in combination with recurly roles). I use RecurlyJS, haven't tested other methods.

Styles and validation from recurlyJS is working, VAT, EUT VAT etc. is working. Paypal was added later and works great too. Example:
https://www.sooperthemes.com/user/18304/subscription/signup/sooperthemes...

markdorison’s picture

Priority: Normal » Major
walangitan’s picture

Extracted the CSS refactor patch from #15. If this is more entangled with the updates on https://www.drupal.org/node/2626662#comment-12003286 then this may need to be re-rolled with the relevant CSS placed in that issue. This patch still need to include the image from #18. Haven't had a chance to take a look at #19 thoroughly yet, though I'm inclined to separate the issues fixed in that patch.

adamzimmermann’s picture

Status: Needs work » Needs review
FileSize
1.45 KB

I believe all of the changes from the patch in #15 have now been applied in other issues. The last remaining item is the generic_card.png file. This patch should add that file.

markdorison’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.