-
Notifications
You must be signed in to change notification settings - Fork 24
[Minecraft Modrinth] Add translations #72
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
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThe pull request adds localization support to the minecraft-modrinth plugin by replacing hardcoded English UI strings with translation lookups throughout the codebase, and introducing English and German translation language files containing the corresponding localized strings. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (3)
87-105: Incomplete localization: table columns remain hardcoded.The table columns (title, author, downloads, date_modified) are using hardcoded identifiers instead of localized labels. Translation keys are already defined in the language files but not being used.
🔎 Proposed fix to localize table column labels
->columns([ ImageColumn::make('icon_url') ->label(''), TextColumn::make('title') + ->label(__('minecraft-modrinth::main.table.columns.title')) ->searchable() ->description(fn (array $record) => (strlen($record['description']) > 120) ? substr($record['description'], 0, 120).'...' : $record['description']), TextColumn::make('author') + ->label(__('minecraft-modrinth::main.table.columns.author')) ->url(fn ($state) => "https://modrinth.com/user/$state", true) ->toggleable(), TextColumn::make('downloads') + ->label(__('minecraft-modrinth::main.table.columns.downloads')) ->icon('tabler-download') ->numeric() ->toggleable(), TextColumn::make('date_modified') + ->label(__('minecraft-modrinth::main.table.columns.date_modified')) ->icon('tabler-calendar') ->formatStateUsing(fn ($state) => Carbon::parse($state, 'UTC')->diffForHumans()) ->tooltip(fn ($state) => Carbon::parse($state, 'UTC')->timezone(user()->timezone ?? 'UTC')->format($table->getDefaultDateTimeDisplayFormat())) ->toggleable(), ])
108-178: Incomplete localization: version entries and actions remain hardcoded.Multiple UI elements in the download modal use hardcoded identifiers instead of localized labels:
- Line 108: Download action label
- Line 135: Version "type" label
- Line 139: "downloads" label
- Line 142: "published" label
- Line 146: "changelog" label
- Line 152: Inner download action label
Translation keys for all these elements exist in the language files.
🔎 Proposed fix to localize version modal labels
->recordActions([ Action::make('download') + ->label(__('minecraft-modrinth::main.actions.download')) ->schema(function (array $record) { $schema = []; /** @var Server $server */ $server = Filament::getTenant(); $versions = array_slice(MinecraftModrinth::getModrinthVersions($record['project_id'], $server), 0, 10); foreach ($versions as $versionData) { $files = $versionData['files'] ?? []; $primaryFile = null; foreach ($files as $fileData) { if ($fileData['primary']) { $primaryFile = $fileData; break; } } $schema[] = Section::make($versionData['name']) ->description($versionData['version_number'] . ($primaryFile ? ' (' . convert_bytes_to_readable($primaryFile['size']) . ')' : ' (' . __('minecraft-modrinth::main.version.no_file_found') . ')')) ->collapsed(!$versionData['featured']) ->collapsible() ->icon($versionData['version_type'] === 'alpha' ? 'tabler-circle-letter-a' : ($versionData['version_type'] === 'beta' ? 'tabler-circle-letter-b' : 'tabler-circle-letter-r')) ->iconColor($versionData['version_type'] === 'alpha' ? 'danger' : ($versionData['version_type'] === 'beta' ? 'warning' : 'success')) ->columns(3) ->schema([ TextEntry::make('type') + ->label(__('minecraft-modrinth::main.version.type')) ->badge() ->color($versionData['version_type'] === 'alpha' ? 'danger' : ($versionData['version_type'] === 'beta' ? 'warning' : 'success')) ->state($versionData['version_type']), TextEntry::make('downloads') + ->label(__('minecraft-modrinth::main.version.downloads')) ->badge() ->state($versionData['downloads']), TextEntry::make('published') + ->label(__('minecraft-modrinth::main.version.published')) ->badge() ->state(Carbon::parse($versionData['date_published'], 'UTC')->diffForHumans()) ->tooltip(Carbon::parse($versionData['date_published'], 'UTC')->timezone(user()->timezone ?? 'UTC')->format('M j, Y H:i:s')), TextEntry::make('changelog') + ->label(__('minecraft-modrinth::main.version.changelog')) ->columnSpanFull() ->markdown() ->state($versionData['changelog']), ]) ->headerActions([ Action::make('download') + ->label(__('minecraft-modrinth::main.actions.download')) ->visible(!is_null($primaryFile)) ->action(function (DaemonFileRepository $fileRepository) use ($server, $versionData, $primaryFile) {
204-229: Incomplete localization: TextEntry labels remain hardcoded.The TextEntry components on lines 204 and 207 use hardcoded English labels ('Minecraft Version' and 'Loader') instead of the translation keys defined in the language files.
🔎 Proposed fix to localize TextEntry labels
Grid::make(3) ->schema([ - TextEntry::make('Minecraft Version') + TextEntry::make('minecraft_version') + ->label(__('minecraft-modrinth::main.page.minecraft_version')) ->state(fn () => MinecraftModrinth::getMinecraftVersion($server) ?? __('minecraft-modrinth::main.page.unknown')) ->badge(), - TextEntry::make('Loader') + TextEntry::make('loader') + ->label(__('minecraft-modrinth::main.page.loader')) ->state(fn () => MinecraftLoader::fromServer($server)?->getLabel() ?? __('minecraft-modrinth::main.page.unknown')) ->badge(), TextEntry::make('installed')
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
minecraft-modrinth/lang/de/main.phpminecraft-modrinth/lang/en/main.phpminecraft-modrinth/src/Enums/ModrinthProjectType.phpminecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.phpminecraft-modrinth/src/MinecraftModrinthPlugin.php
🧰 Additional context used
🧬 Code graph analysis (1)
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (1)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (2)
fromServer(29-45)getLabel(13-19)
🪛 GitHub Actions: Lint
minecraft-modrinth/lang/en/main.php
[error] 1-1: Pint style issues detected: no_whitespace_in_blank_line, single_blank_line. Laravel check failed. Process completed with exit code 1.
minecraft-modrinth/lang/de/main.php
[error] 1-1: Pint style issues detected: no_whitespace_in_blank_line, single_blank_line. Laravel check failed. Process completed with exit code 1.
🔇 Additional comments (8)
minecraft-modrinth/src/Enums/ModrinthProjectType.php (1)
16-17: LGTM!The localization implementation is correct. Both translation keys are properly defined in the language files.
minecraft-modrinth/src/MinecraftModrinthPlugin.php (2)
34-34: LGTM!The label is correctly localized using the translation key defined in the language files.
47-47: LGTM!The notification title is correctly localized.
minecraft-modrinth/src/Filament/Server/Pages/MinecraftModrinthProjectPage.php (5)
128-128: LGTM!The version description correctly uses the localized string for "no file found".
159-159: LGTM!Download notification titles are properly localized.
Also applies to: 167-167
190-190: LGTM!The folder link label is correctly localized with parameter substitution.
205-205: LGTM!The "unknown" fallback values are properly localized.
Also applies to: 208-208, 226-226
211-211: LGTM!The installed count label is correctly localized with parameter substitution.
Boy132
left a comment
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.
Please use trans() instead of __()
|
okay weit |
…hProjectType and MinecraftModrinthPlugin
…ResourcePage" This reverts commit ecfc55f.
Boy132
left a comment
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.
Thank you for your contribution!
For future reference, please create a separate branch for pull requests and don't use main. :)
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.