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.

Sign up for GitHub

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

Jump to bottom

Fixes variant switching in preview mode #120

Closed
wants to merge 2 commits into from
Closed

Fixes variant switching in preview mode #120

wants to merge 2 commits into from

Conversation

Crease29
Copy link

  • Removes unnecessary div in form
  • Adds preview parameter as hidden input because switching variants in preview mode is broken

@CLAassistant
Copy link

CLAassistant commented Mar 10, 2018 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@Sieg Sieg self-assigned this Mar 22, 2018
@Sieg
Copy link
Member

Sieg commented Mar 22, 2018

Hi @Crease29, Could you please link the related bug, or write more information to reproduce the Issue? It looks working fine.

Sieg
Sieg reviewed Mar 22, 2018
View reviewed changes
tpl/page/details/inc/productmain.tpl Outdated
<input type="hidden" name="cnid" value="[{$oView->getCategoryId()}]">
<input type="hidden" name="anid" value="[{if !$oDetailsProduct->oxarticles__oxparentid->value}][{$oDetailsProduct->oxarticles__oxid->value}][{else}][{$oDetailsProduct->oxarticles__oxparentid->value}][{/if}]">
<input type="hidden" name="actcontrol" value="[{$oViewConf->getTopActiveClassName()}]">
[{if $oConf->getRequestParameter('preview')}]
Copy link
Member

Choose a reason for hiding this comment

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

There is no $oConf available here :/

@Crease29
Copy link
Author

Thank you for your review @Sieg .

Steps to reproduce:

  1. Login to the administration interface
  2. Open an inactive master article that has variants with the preview link at the bottom of the screen
  3. Select a variant in the frontend. There should appear an error without my PR.

@Sieg
Copy link
Member

Sieg commented Mar 22, 2018

Hm, yes, I have tried it, works well for me on EE. Will check CE again, anything you can share from EXCEPTION log? Anything custom in shop?

@Crease29
Copy link
Author

No modules, nothing in the EXCEPTION_LOG. The error for me was, that it just says that the article can't be found, which is obvious because the preview parameter was missing in the request that loads the selected variant and as the article is inactive it shall not appear in the frontend.

@Crease29
Copy link
Author

Here you can see it in the demo shop: https://www.youtube.com/watch?v=x__B_phDuag

Sieg added a commit that referenced this pull request Mar 22, 2018
@Sieg
PR-120 Update changelog
540e043 
Related #120
@Sieg
Copy link
Member

Sieg commented Mar 22, 2018

Thanks! That "active" checkbox was the question :) Have merged your PR recently. Have a nice day!

@Sieg Sieg closed this Mar 22, 2018
@Sieg Sieg removed the in progress label Mar 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sieg Sieg Sieg left review comments

Assignees

@Sieg Sieg

Labels
Bug Ready to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Crease29 @CLAassistant @Sieg