Describe your bug or feature request.

Payment Gateway plugins can define js_library to be attached in different scenarios when the payment gateway option is available for selection. That feature is used by Braintree among other payment modules. The plugin key was added in #2854852: Payment gateway JS is not always loaded.

While this feature is working just well it doesn't fully reflect what are the possibilities therefore the naming can be a bit confusing. Ie. the library doesn't have to only feature JS scripts, it can also feature CSS stylesheets (for example with gateway branding). It can even feature CSS without JS.

Furthermore, there is no reason why plugins should be limited to feature only one library. While additional libraries could still be defined and attached via library dependency we are missing the possibility to conditionally load some libraries, ie. based on plugin configuration. Having the ability to add multiple libraries (mix of CSS ad JS) could help resolve some long-standing issues like #3183931: Display credit card icons in the checkout form. It could also help Payment Gateway to provide branding that should be featured along with the payment method option in the checkout form.

CommentFileSizeAuthor
Screenshot from 2024-08-05 09-50-42.png25.87 KBzaporylie

Issue fork commerce-3465875

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

zaporylie created an issue. See original summary.

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented

Proposed solution:

  • Add libraries key to plugin definition.
  • Mark js_library as deprecated.
  • Append library defined in js_library to the array of libraries for backward compatibility
  • Adjust code attaching libraries to follow the new format (libraries are arrays of strings, not strings).
jsacksick’s picture

jsacksick commented

Sounds like a good improvement to me, would you like to work on this?

zaporylie opened merge request !295

zaporylie’s picture

zaporylie
he/him
Primary language Polish
commented
Status: Active » Needs review

Just added the MR.

jsacksick’s picture

jsacksick commented

I'm thinking that we should deprecate the $js_library property as well (both in the annotation and the attribute, and as discussed in Slack, wondering if we should adopt the deprecated format that includes the version number.
Basically, deprecated in Commerce 3.0.x and removed from Commerce 4.0.x perhaps? Or Commerce 3.1.x? Not sure.

jsacksick’s picture

jsacksick commented
Status: Needs review » Fixed

Thank you for this, merged!

jsacksick’s picture

jsacksick commented

I was writing a change record for this change, but noticed the actual core issue referenced has been fixed since D9.4.5... Do we still need the workaround/fix, or perhaps we still need this mechanism in place so there is no need to add the library as part of the render array?

Status: Fixed » Closed (fixed)

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