Skip to content

fix: Use navigation.group configuration in MailResource#59

Merged
Baspa merged 1 commit intobackstagephp:mainfrom
HelgeSverre:fix/navigation-group-config
Sep 26, 2025
Merged

fix: Use navigation.group configuration in MailResource#59
Baspa merged 1 commit intobackstagephp:mainfrom
HelgeSverre:fix/navigation-group-config

Conversation

@HelgeSverre
Copy link
Copy Markdown
Contributor

Summary

  • Fixes the MailResource to use the filament-mails.navigation.group configuration option
  • Previously the navigation group was hardcoded to __('Emails') ignoring the config
  • Now reads from config with a fallback to the previous default value

Details

The issue was in src/Resources/MailResource.php:61 where getNavigationGroup() returned a hardcoded value instead of checking the configuration.

Before:

public static function getNavigationGroup(): ?string
{
    return __('Emails');
}

After:

public static function getNavigationGroup(): ?string
{
    return config('filament-mails.navigation.group', __('Emails'));
}

Test plan

  • Verified the configuration option exists in config/filament-mails.php
  • Confirmed EventResource and SuppressionResource already properly reference MailResource's navigation group
  • Change maintains backward compatibility with fallback to previous default

Closes #58

🤖 Generated with Claude Code

The MailResource was hardcoded to use '__('Emails')' as the navigation group,
ignoring the 'filament-mails.navigation.group' configuration option.

This change updates the getNavigationGroup() method to read from the config
with a fallback to the previous default value, allowing users to customize
the navigation group as intended.

Fixes backstagephp#58

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@Baspa Baspa self-assigned this Sep 26, 2025
@Baspa Baspa added the bug Something isn't working label Sep 26, 2025
@Baspa Baspa merged commit 24d5d50 into backstagephp:main Sep 26, 2025
5 checks passed
@Baspa
Copy link
Copy Markdown
Contributor

Baspa commented Sep 26, 2025

Fixed in v3.0.4, thanks @HelgeSverre!

@arshaviras
Copy link
Copy Markdown

arshaviras commented Oct 2, 2025

Hi @markvaneijk , this method not works!

public static function getNavigationGroup(): ?string
    {
        return config('filament-mails.navigation.group', __('Emails'));
    }

In Laravel, config('key', $default) only uses $default if the key is missing - not when it exists and is null.

So config('filament-mails.navigation.group', __('Emails')) resolves to null, which means no group header.

I think you should use like this

public static function getNavigationGroup(): ?string
    {
        return __(config('filament-mails.navigation.group') ?? 'Emails');
    }

If need I can create PR for this.
Thanks

@HelgeSverre HelgeSverre deleted the fix/navigation-group-config branch October 2, 2025 10:04
@arshaviras
Copy link
Copy Markdown

arshaviras commented Oct 3, 2025

@markvaneijk, @Baspa this will not work, it will thrown error - __() helper can't used in config files, but we need to use multilingual key. So my version described above resolves an issue.

'navigation' => [
        'group' => __('Emails'),
        'sort' => 9,
    ],

@markvaneijk
Copy link
Copy Markdown
Contributor

You are correct @arshaviras, we've released v3.0.6 with a fix for this and added some extra configuration options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Config navigation -> group is not working

4 participants