The font size (which is currently hard-coded in uc_stripe.js to 24px) of the new Stripe.js Elements input is too large for small displays and results in overlapping text.

I recommend at the very least to remove this option and use the default provided by Stripe (which is 1em and works much better as a default). Or, make the "style" options an alterable thing.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

joelstein created an issue. See original summary.

joelstein’s picture

Status: Active » Needs work
FileSize
350 bytes

Here's a patch which simply removes the hard-coded fontSize option.

AndraeRay’s picture

Thanks for catching this.

How about a (hard coded) 18px default? I tested it and it works even for a screen as small as 360px. I think that's the smallest reasonable mobile screen size. (https://gs.statcounter.com/screen-resolution-stats/mobile/worldwide)

1em can be hard on the eyes especially since it uses the default gray from stripe.

In the future if there really is a desire to edit the settings, that can be a separate feature request.

joelstein’s picture

My opinion is that we should not hard code any style-related CSS stuff in the way this is presented, unless also making it at least alterable through a hook. 18px is still too big for my use case, at least. Also, Stripe's defaults are reasonable, if not perfect, and I think the module should use their defaults by default. :)

But, if there was a way to alter the JS options that get passed to Stripe Elements, then it would make it easier for themers to adjust the presentation to match their website design requirements.

AndraeRay’s picture

@joelstein, good point.

Let me see where an alter hook can be added.

AndraeRay’s picture

I've been able to spend some time on this, and I've created a patch.

The default font size is now 18px, and FontSize, and colors are now editable. You can even add additional styles found in the stripe docs:
https://stripe.com/docs/stripe-js/reference#element-options

You can edit the settings from the Stripe settings page.

It worked fine from my testing, but I would appreciate a second check.

AndraeRay’s picture

Status: Needs work » Needs review
rajeev_drupal’s picture

Assigned: Unassigned » rajeev_drupal
rajeev_drupal’s picture

Tested. Working fine for me.

rajeev_drupal’s picture

Assigned: rajeev_drupal » Unassigned
AndraeRay’s picture

Thanks for testing @rajeev_drupal

AndraeRay’s picture

@joelstein, you're right. Good observation, 18px can still be too big.

I have set the default to be 1em to make it easier for everyone.

Here is a new patch, it's almost identical to the patch from #6. I'll go ahead and commit to dev branch.

AndraeRay’s picture

Title: Remove font size declaration (or make it alterable) » Stripe element text overlaps on small displays

  • AndraeRay committed 4ee41b1 on 7.x-3.x
    Issue #3081576 by AndraeRay, joelstein, rajeev_drupal: Stripe element...
AndraeRay’s picture

Status: Needs review » Fixed

Thanks @rajeev_drupal and @joelstein

Status: Fixed » Closed (fixed)

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

AndraeRay’s picture