Skip to content

Translations for the menu #360

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

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

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

Successfully merging this pull request may close these issues.

2 participants