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

Module tab new subtree #8814

Merged

Conversation

@Quetzacoalt91
Copy link
Member

commented Mar 1, 2018

Questions Answers
Branch? 1.7.3.x
Description? This PRs improves #5922, allowing module contributors to add new entries in the menu. It fixes an issue when he wants to add a new level of submenus (for instance, to display tabs in the header of the BO on an existing page from the core).
Type? improvement
Category? BO
BC breaks? Nope
Deprecations? Nope
Fixed ticket? http://forge.prestashop.com/browse/BOOM-4912
How to test? Install the following module: https://github.com/Quetzacoalt91/moduletab, you must discover new tabs on the Theme page.

capture du 2018-02-28 15-13-37


This change is Reviewable

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:module-tab-new-subtree branch from d27a210 to db41a43 Mar 1, 2018

* @param $idParent
* @return array
*/
public function findByParentId($idParent)

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Mar 2, 2018

Member

findByIdParent($idParent) should work without need to create it :)

This comment has been minimized.

Copy link
@Quetzacoalt91

Quetzacoalt91 Mar 2, 2018

Author Member

That was only for the IDE autocompletion, should I remove it?

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Mar 2, 2018

Member

nope, you're right :)

/**
* Find the parent ID from the given tab context
*
* @param ParameterBag $data The structure of the tab.

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Mar 8, 2018

Member

can we use a better name for this variable? $tabStructure maybe.?

$idParent = 0;
$parentClassName = $data->get('parent_class_name', $data->get('ParentClassName'));
if (!empty($parentClassName)) {
// Could be a previously duicated tab

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Mar 8, 2018

Member
- duicated
+ duplicated
$parentClassName = $data->get('parent_class_name', $data->get('ParentClassName'));
if (!empty($parentClassName)) {
// Could be a previously duicated tab
$idParent = (int)$this->tabRepository->findOneIdByClassName($parentClassName.self::SUFFIX);

This comment has been minimized.

Copy link
@mickaelandrieu

mickaelandrieu Mar 8, 2018

Member

maybe only cast the last call of $idParent in duplicateParentIfAlone function?

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:module-tab-new-subtree branch from cdbeefb to 26f5e47 Mar 14, 2018

@LittleBigDev
Copy link
Contributor

left a comment

Approving again after PR was rebased

@mickaelandrieu

This comment has been minimized.

Copy link
Member

commented Mar 29, 2018

ping @eternoendless ready for which milestone? 1.7.3.2?

@eternoendless

This comment has been minimized.

Copy link
Member

commented Apr 4, 2018

I think this should be in 1.7.4.0
@Quetzacoalt91 could you please rebase onto develop?

@eternoendless eternoendless added this to the 1.7.4.0 milestone Apr 4, 2018

@Quetzacoalt91

This comment has been minimized.

Copy link
Member Author

commented Apr 4, 2018

@Quetzacoalt91 Quetzacoalt91 changed the base branch from 1.7.3.x to develop Apr 11, 2018

@Quetzacoalt91 Quetzacoalt91 force-pushed the Quetzacoalt91:module-tab-new-subtree branch from 26f5e47 to 80f4aaf Apr 11, 2018

@Quetzacoalt91

This comment has been minimized.

Copy link
Member Author

commented Apr 11, 2018

Rebased over develop

@eternoendless

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

@marionf could you please re-test it?

@marionf

This comment has been minimized.

Copy link
Contributor

commented Apr 11, 2018

it's still ok for me @eternoendless :)

@mickaelandrieu mickaelandrieu merged commit 228059d into PrestaShop:develop Apr 11, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@mickaelandrieu

This comment has been minimized.

Copy link
Member

commented Apr 11, 2018

Thanks everyone!

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.