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
Landing page builder feature #661
Conversation
…feat-landing-page-builder # Conflicts: # Block/Algolia.php # view/adminhtml/templates/adminjs.phtml # view/adminhtml/templates/analytics/overview.phtml
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here are some changes to the wording we've seen with Lucia + some questions that we can discuss if available.
Let us know if you have any questions.
Thank you!
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
view/adminhtml/ui_component/algolia_algoliasearch_landingpage_form.xml
Outdated
Show resolved
Hide resolved
Hey @damcou, TravisBuddy Request Identifier: 62b16d90-2e2a-11e9-93d2-519d0ef52680 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
The PR is huge, I probably didn't get all the details.
I noticed only 2 things I wanted to ask, so I added questions. Otherwise it looks good :)
Good job, @damcou 🙌
/** @var \Algolia\AlgoliaSearch\Model\LandingPage $landingPage */ | ||
$landingPage = $this->landingPageFactory->create(); | ||
$landingPage->getResource()->load($landingPage, $landingPageId); | ||
$landingPage->getResource()->delete($landingPage); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
URL is deleted automatically?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I added a _afterDelete() method on the resource model : https://github.com/algolia/algoliasearch-magento-2/blob/feat-landing-page-builder/Model/ResourceModel/LandingPage.php#L79
$resultPage->getConfig()->getTitle()->prepend($breadMain); | ||
|
||
$dataPersistor = $this->_objectManager->get(\Magento\Framework\App\Request\DataPersistorInterface::class); | ||
$dataPersistor->clear('landing_page'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is data persistor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it's used to keep the submitted data when needed : https://github.com/algolia/algoliasearch-magento-2/blob/feat-landing-page-builder/Controller/Adminhtml/Landingpage/Save.php#L119.
I took inspiration from the M2 CMS page feature for this : https://github.com/magento/magento2/blob/2.3-develop/app/code/Magento/Cms/Controller/Adminhtml/Page/Index.php#L56
Hey @damcou, TravisBuddy Request Identifier: b45fc0d0-3111-11e9-91d5-11294258c2e5 |
No description provided.