Skip to content

Translations for the menu #360

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

EmilsM
Copy link

@EmilsM EmilsM commented Feb 26, 2025

Added an option with a dynamic row to translate menu items.
Also works in import/export
Also works with "Import from Categories"
code might be a little rough & need improvements, as this was done in a few hours.

Comment on lines +194 to +216

// Create store view lookup array
$storeViewLabels = [];
$websites = $this->systemStore->getWebsiteCollection();
$websiteNames = [];

// Create website name lookup array
foreach ($websites as $website) {
$websiteNames[$website->getId()] = $website->getName();
}

foreach ($this->systemStore->getStoreCollection() as $store) {
if ($store->isActive()) {
$websiteName = isset($websiteNames[$store->getWebsiteId()])
? $websiteNames[$store->getWebsiteId()]
: '';
$storeViewLabels[$store->getId()] = [
'value' => $store->getId(),
'label' => sprintf('%s -> %s', $websiteName, $store->getName())
];
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like duplicate piece of code. I think you can use the function getStoreViews().

->setStoreId($storeId)
->setTitle($title);

$this->nodeTranslationRepository->save($translation);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's worth to wrap up $this->nodeTranslationRepository->save($translation); in try-catch block

@@ -45,4 +52,14 @@ public function get($storeCode): ?StoreInterface

return $store;
}

public function getIdByCode(string $storeCode): ?int
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this function called somewhere? I don't see any usages.

/**
* @inheritDoc
*/
public function getTranslationId(): ?int
Copy link
Contributor

Choose a reason for hiding this comment

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

I think a declaration of the function getTranslationId() is missing in the NodeTranslationInterface

/**
* @inheritDoc
*/
public function setTranslationId(int $id): NodeTranslationInterface
Copy link
Contributor

@maartynaa maartynaa May 6, 2025

Choose a reason for hiding this comment

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

Same here and for getCreatedAt(), setCreatedAt(), getUpdatedAt(), setUpdatedAt()

Comment on lines +118 to +126
public function getValue(): string
{
return (string)$this->getData('value');
}

public function setValue(string $value): void
{
$this->setData('value', $value);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these functions called somewhere? I don't see any usages, but please double check.

namespace Snowdog\Menu\Plugin\Model\Menu\Node;

use Snowdog\Menu\Api\NodeTranslationRepositoryInterface;
use Snowdog\Menu\Api\Data\NodeTranslationInterface;
Copy link
Contributor

Choose a reason for hiding this comment

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

The class Snowdog\Menu\Api\Data\NodeTranslationInterface is imported but never used

@@ -1,5 +1,5 @@
{
"name": "snowdog/module-menu",
"name": "magebitcom/snowdog-module-menu",
Copy link
Contributor

@maartynaa maartynaa May 6, 2025

Choose a reason for hiding this comment

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

Please don't overwrite module name in Snowdog repository

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants