Skip to content

Conversation

progreg
Copy link
Contributor

@progreg progreg commented Sep 15, 2018

Description

Added default handle if result count of search results is zero. The feature was implemented for both search variations. For a simple search, the handle will be catalogsearch_result_index_noresults and for advanced search, it will be catalogsearch_advanced_result_noresults.

Fixed Issues (if relevant)

  1. Add a custom layout handle when there are no search results #18038: Add a custom layout handle when there are no search results

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@magento-cicd2
Copy link
Contributor

magento-cicd2 commented Sep 15, 2018

CLA assistant check
All committers have signed the CLA.

@magento-engcom-team
Copy link
Contributor

Hi @progreg. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento-engcom-team give me test instance - deploy test instance based on PR changes
  • @magento-engcom-team give me $VERSION instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

@okorshenko
Copy link
Contributor

@magento-engcom-team give me test instance

@magento-engcom-team
Copy link
Contributor

Hi @okorshenko. Thank you for your request. I'm working on Magento instance for you

@magento-engcom-team
Copy link
Contributor

Hi @okorshenko, here is your new Magento instance.
Admin access: https://pr-18069.engcom.dev.magento.com/admin
Login: admin Password: 123123q

@ishakhsuvarov ishakhsuvarov modified the milestone: Release: 2.3.1 Sep 18, 2018
@jakwinkler
Copy link
Contributor

I'm happy that PR was created to this but... it does not work as expected.
I've implemented this pull request to 2.2.5 version (EE), created a file catalogsearch_result_noresults.xml in Magento CatalogSearch module with following content

<?xml version="1.0"?>
<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" layout="1column"
   xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <referenceContainer name="header" remove="true" />
    <referenceContainer name="header-wrapper" remove="true" />
    <referenceContainer name="footer" remove="true" />
</page>

Am I doing something wrong? Basically the layout xml file is not processed at all... at least thats what I can tell.

For now I will simply create a redirect to a new controller where layout handles work fine.

@ishakhsuvarov
Copy link
Contributor

Hi @progreg
Could you please take a look at the comments above?

Thanks!

@progreg
Copy link
Contributor Author

progreg commented Sep 21, 2018

@qsolutions-pl, thanks for response. I will check it.

@progreg
Copy link
Contributor Author

progreg commented Sep 25, 2018

Hi @qsolutions-pl, the issue was fixed and now you can check again it by using next handles:

  • catalogsearch_result_index_noresults
  • catalogsearch_advanced_result_noresults

@jakwinkler
Copy link
Contributor

@progreg I do see layout handles being used when I implemented this pull request to my magento installation. Even tests pass when I run them.
But... frontend page still uses 2 columns instead of 1 - that is what I'm trying to accomplish - cache cleared, di cleared :(
To solve this I've created a plugin on Result block and redirect user to a new controller action for now.

@ishakhsuvarov
Copy link
Contributor

Hi @qsolutions-pl
Looks like you already have a solution that is working for you. Feel free to submit it with another Pull Request or even push additional commits to this one.

@progreg
Copy link
Contributor Author

progreg commented Oct 1, 2018

@qsolutions-pl, yeap, you are right. I will take a look again maybe i missed something. Thanks for response

@josefbehr josefbehr self-assigned this Oct 7, 2018
@progreg
Copy link
Contributor Author

progreg commented Oct 8, 2018

@qsolutions-pl, problem with layout was fixed. Please take a look if you still interested in such improvement. Thanks

@progreg progreg assigned progreg and josefbehr and unassigned josefbehr Oct 8, 2018
@progreg progreg added Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release and removed Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Nov 14, 2018
@sivaschenko sivaschenko self-assigned this Nov 21, 2018
Copy link
Member

@sivaschenko sivaschenko left a comment

Choose a reason for hiding this comment

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

@progreg thanks for the contribution! Can you please add phpdocs to all added constants and methods.

$this->assertEquals($redirectResultMock, $instance->execute());
}

public function testNoResultsHandle()
Copy link
Member

Choose a reason for hiding this comment

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

please add docblocks/comments for all introduced constants and methods

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@magento-engcom-team
Copy link
Contributor

Hi @sivaschenko, thank you for the review.
ENGCOM-3526 has been created to process this Pull Request

@magento-engcom-team
Copy link
Contributor

Hi @progreg. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.

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

Successfully merging this pull request may close these issues.

8 participants