in 8.1.x, commerce_stripe.libraries.yml there is a dependency to classy

messages:
version: VERSION
js:
js/commerce_stripe.messages.js: {}
dependencies:
- classy/messages
- core/jquery
- core/drupal

Classy being removed from D10, classy/messages should also be removed and let themes manage display of '

'

CommentFileSizeAuthor
#11 error_messages.png200.54 KBvmarchuk
#8 commerce_stripe-3366832-8.patch638 bytesgrimreaper

Issue fork commerce_stripe-3366832

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

kiwad created an issue. See original summary.

elber made their first commit to this issue’s fork.

elber opened merge request !26

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Active » Needs review
kiwad’s picture

kiwad commented

That was fast :)

I've tested with normal test card and insufficent funds test card it removes

User warning : The following theme is missing from the file system: classy dans Drupal\Core\Extension\ExtensionPathResolver->getPathname()

and

Deprecated function : dirname(): Passing null to parameter #1 ($path) of type string is deprecated dans Drupal\Core\Extension\ExtensionPathResolver->getPath()

in /checkout/{id}/review and checkout/{id}/order_information

johnpitcairn’s picture

johnpitcairn commented
Status: Needs review » Needs work
grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
Status: Needs work » Reviewed & tested by the community

Hi,

Thanks for the MR, I encountered the same issue and it fixed it.

grimreaper’s picture

grimreaper
Primary language French
Location France 🇫🇷
commented
StatusFileSize
new commerce_stripe-3366832-8.patch638 bytes

Uploading patch for easier Composer usage.

c_archer’s picture

c_archer commented

Patch works for me on D10

rszrama’s picture

rszrama commented

I've noticed this issue during testing in a clean Commerce Kickstart 3.x instance as well.

vmarchuk’s picture

vmarchuk
Primary language English
Location Rivne, UA
commented
StatusFileSize
new error_messages.png200.54 KB

@rszrama

I also noticed that we lost styles for error messages on D10. See the attached screenshot for more information.
Error messages

So it seems we need to create our own styles if we don't want to have dependencies on other themes or modules.
Also, I see a dependency on "classy/messages" in the commerce_braintree module (we need to create an issue there), so maybe it makes sense to create global styles?

rszrama’s picture

rszrama commented
Title: Replace classy dependency » Remove the Classy dependency for D10 compatibility
Category: Bug report » Task
Status: Reviewed & tested by the community » Needs work

Ok, since this change simply removes styling that should be there, it's not an appropriate fix. Instead, we've created an issue against Commerce Core for defining a replacement library that this module can then be updated to use: #3385414: Provide a basic replacement for Classy's messages library in the Payment module

vmarchuk opened merge request !60

vmarchuk’s picture

vmarchuk
Primary language English
Location Rivne, UA
commented
Status: Needs work » Needs review

Opened MR with the fixes. Since we don't know in which version of commerce the payment_messages library will be available, we need to add it dynamically (check if it exists and then add it).

rszrama’s picture

rszrama commented

Are we sure the string of ../../../ there on image URLs is safe? Does core use that same pattern? Just concerned that we can't actually guarantee the module's placement in a directory structure and should probably copy images to our module instead.

vmarchuk’s picture

vmarchuk
Primary language English
Location Rivne, UA
commented

@rszrama

Are we sure the string of ../../../ there on image URLs is safe? Does core use that same pattern? Just concerned that we can't actually guarantee the module's placement in a directory structure and should probably copy images to our module instead.

Yes, you're right. I updated the MR here https://www.drupal.org/project/commerce/issues/3385414#comment-15295716

sagesolutions’s picture

sagesolutions commented
Status: Needs review » Reviewed & tested by the community

Adding the styling is now completed in the main commerce module, specifically https://www.drupal.org/project/commerce/issues/3385414#comment-15298522

The patch #8 fixes the classy dependency error. Alternatively, MR #26 fixes the error too. Both fixes simply remove the dependency.

bserem’s picture

bserem commented

+1, this takes care of all the warnings

liliplanet’s picture

liliplanet commented

Awesome thank you #8 removes the error

leducdubleuet’s picture

leducdubleuet commented

Now that the fix in the issue #3385414 has landed in Commerce core 2.37, it would be great to have a new release including this fix please.

Thank you!

TomTech made their first commit to this issue’s fork.

c_archer’s picture

c_archer commented

  • TomTech committed 8cc62416 on 8.x-1.x authored by vmarchuk 
    Issue #3366832 by vmarchuk, elber, TomTech, Grimreaper, kiwad, rszrama,...
tomtech’s picture

tomtech commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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