Skip to content

Fix: [FO]Carrier extra content not work on deliveryFormSelector change#496

Merged
nicosomb merged 5 commits intoPrestaShop:developfrom
GytisZum:carrier-dynamic-extra-content-fix
May 23, 2023
Merged

Fix: [FO]Carrier extra content not work on deliveryFormSelector change#496
nicosomb merged 5 commits intoPrestaShop:developfrom
GytisZum:carrier-dynamic-extra-content-fix

Conversation

@GytisZum
Copy link
Contributor

@GytisZum GytisZum commented May 5, 2023

Questions Answers
Description? Previously delivery extra content was shown only on page load. Added Prestashop event for extra content to be dynamic, tried to recreate the slide down animation as close as in classic theme with content max-heigh property.
Type? bug fix
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #478
How to test? Add product to cart. Sign in and complete step 1 and step 2 on checkout. On delivery option step select different deliveries. Make sure delivery option has module with has an hook displayCarrierExtraContent which add's extra content.

On page load:
image

Selecting method without extra content:
image

Selecting another method which has extra content:
image

Copy link
Contributor

@ga-devfront ga-devfront left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

really good work, thank you !

@hibatallahAouadni hibatallahAouadni self-assigned this May 9, 2023
Copy link

@hibatallahAouadni hibatallahAouadni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @GytisZum

Thanks for your PR 🙏
Unfortunately, it seems that the issue still not fixed with your PR, see the attached screen record below:

hb-pr_496.webm

Please check and feedback.

Thanks!

@GytisZum
Copy link
Contributor Author

GytisZum commented May 11, 2023

Hello,

Thank you for response and the video you provided, however I am unable to recreate same issue on different PrestaShop versions (8.1.0 | 8.0.2).
Could you please confirm that assets was builded as this PR have style file which has to be compiled. From video you provided it looks like they are missing that stylesheet.
If it is not the case could you please double check if it is not the cache issues.
If issue still appears please provide me your environment you test with.

  • PrestaShop version.
  • Browser.
  • Operating system.
  • Module you test with.

Please check and feedback
Thank you!

Copy link

@hibatallahAouadni hibatallahAouadni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @GytisZum

On develop branch ❌ , I still encounter the same issue as you can see the first part of the attached screen record:

  • PrestaShop version: develop
  • Browser: Chrome 107.0.5304.87
  • Operating system: Ubuntu 22.04.1 LTS
  • Module you test with: tox_extracarrier

While with 8.1.x branch ✔️ , the issue is fixed as you can see the second part of the attached screen record:

  • PrestaShop version: 8.1.x
  • Browser: Chrome 107.0.5304.87
  • Operating system: Ubuntu 22.04.1 LTS
  • Module you test with: tox_extracarrier
pr_496.webm

Please check and feedback.

Thanks!

@hibatallahAouadni
Copy link

Please find attached the module I used to test this PR (developed by @Hlavtox )

tox_extracarrier.zip

@guizme555
Copy link

Hello @hibatallahAouadni ,
I just tested on the develop branch and it works well (and I did clear my browser cache).
My configuration :

  • PrestaShop version: develop
  • Browser: Chrome Version 113.0.5672.92 (Build officiel) (arm64)
  • Operating system: Mac OS Ventura 13.3.1 (a) (22E772610a)
  • Module you test with: tox_extracarrier

Copy link

@sallemiines sallemiines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @GytisZum

Thanx for the PR !
Pr checked with develop , PS804 & PS810-rc => OK

810RC.mp4

LGTM ! QA ✔️

Copy link

@hibatallahAouadni hibatallahAouadni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @GytisZum

LGTM too, it seems to be a cache issue on my side 😓

pr_496.webm

As you can see it works on develop branch too.

Thanks!

@nicosomb nicosomb added this to the Beta milestone May 23, 2023
@nicosomb nicosomb merged commit 27c96d3 into PrestaShop:develop May 23, 2023
@nicosomb
Copy link
Contributor

thank you @GytisZum !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Carrier extra content not work on deliveryFormSelector change

7 participants