Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only disable following steps in the checkout process when the current step has a continue button #15042

Merged
merged 1 commit into from Aug 12, 2019

Conversation

@jolelievre
Copy link
Contributor

commented Aug 8, 2019

Questions Answers
Branch? 1.7.6.x
Description? In the checkout process only disable following steps when the current step has a continue button This will prevent to block the workflow in themes which don't have the continue button in the "Personal information" step (when logged in)
Type? bug fix
Category? FO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #14846
How to test? First you need to be logged in with a PrestaShop frontend user, then in the checkout process when you are (at least) at the address step you can go backward by clicking on the "Personal information" title

Now this bug only occurs if you have a theme which doesn't include the continue button, so to simulate this you need to edit the template themes/classic/templates/checkout/_partials/steps/personal-information.tpl and remove continue button (the one with class continue)

When you refresh and retry the previous actions on checkout page you will see that when you click on "Personal Information" title, the next steps are clickable but as soon as you go to the next step you need to click on Continue
So this fix shouldn't break the required change from #13126

This change is Reviewable

@jolelievre jolelievre added this to the 1.7.6.1 milestone Aug 8, 2019
@jolelievre jolelievre requested a review from PrestaShop/prestashop-core-developers as a code owner Aug 8, 2019
Copy link
Contributor

left a comment

lgtm

@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

Hi @jolelievre,

In this case:

  1. you are just logged in with a PrestaShop frontend user

  2. Add some products to the cart

  3. go to the checkout process

  4. You are in "Addresses Step"

  5. Click on the "Personal Information" step

  6. The step "Addresses" is not disabled => OK but the edit button is not displayed
    image

  7. but, you cannot click on it => Error

  8. if you reload the page => you are in the "Addresses Step" again

  9. Click on Continue button => you are on the "Shipping Method" step

  10. Click on the "Personal Information" step

  11. The step "Addresses" is not disabled => OK & The Edit button is displayed

  12. So you can click on the "Adress" Step
    image

Thanks!

@jolelievre

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

@khouloudbelguith
thanks, seems like I didn't test enough the use case when address is not set yet. I'm gonna try it!

… step has a continue button
@jolelievre

This comment has been minimized.

Copy link
Contributor Author

commented Aug 9, 2019

@khouloudbelguith
Ok I fixed the bug, you can now click on Addresses when no "continue" button is present in the "Personal informations"

About the other steps you mentioned, when a step has already been completed and you click on continue it is logical that you move to the latest step, unless your modification requires for a change in another step (for example if you change your address you might need to redefine your shipping method)

@jolelievre jolelievre force-pushed the jolelievre:manage-step-without-continue branch from 93f7af9 to 2974dd7 Aug 9, 2019
@eternoendless eternoendless changed the title In the checkout process only disable following steps when the current… Only disable following steps in the checkout process when the current step has a continue button Aug 9, 2019
@khouloudbelguith

This comment has been minimized.

Copy link
Contributor

commented Aug 9, 2019

@jolelievre, thanks!
It is OK now.
Waiting for review

const $nextSteps = $clickedStep.nextAll();
$nextSteps.addClass('-unreachable').removeClass('-complete');
$('.step-title', $nextSteps).addClass('not-allowed');
}

This comment has been minimized.

Copy link
@eternoendless

eternoendless Aug 9, 2019

Member

Consider extracting functions and objects, this code is very complicated to understand.

Picture this:

if (!clickedStep.isUnreachable()) {
	clickedStep.makeCurrent();
	if (clickedStep.hasContinueButton()) {
		steps.disableAllAfter(clickedStep)
	} else {
        steps.enableAllBefore(clickedStep)
    }
}

Looks better isn't it?

This comment has been minimized.

Copy link
@jolelievre

jolelievre Aug 9, 2019

Author Contributor

Yep much better, but then I need to extend jQuery functions? I'm not familiar with it but I can try
Or could this be enough:

if (!isUnreachable(clickedStep)) {
	makeCurrent(clickedStep);
	if (hasContinueButton(clickedStep)) {
		disableAllAfter(steps, clickedStep)
	} else {
        	enableAllBefore(steps, clickedStep)
    	}
}
@matks matks added waiting for QA and removed waiting for QA labels Aug 12, 2019
@matks
matks approved these changes Aug 12, 2019
@matks

This comment has been minimized.

Copy link
Contributor

commented Aug 12, 2019

Requested change from @eternoendless will be handled for 1.7.7
Current PR will be merged for 1.7.6.x as it has been validated by @khouloudbelguith

@matks matks merged commit c2bd5e0 into PrestaShop:1.7.6.x Aug 12, 2019
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.