Skip to content

Conversation

@yassersebai
Copy link

  • Add union types for navigationGroup and navigationIcon
  • Maintains backward compatibility
  • Fixes PHP Fatal Error when setting navigation properties
  • Tested with Laravel 11 + Filament v4

- Add union types for navigationGroup and navigationIcon
- Maintains backward compatibility
- Fixes PHP Fatal Error when setting navigation properties
- Tested with Laravel 11 + Filament v4
@ManukMinasyan ManukMinasyan requested a review from Copilot August 31, 2025 10:20
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds type declarations for navigation properties to fix compatibility issues with Filament v4. The changes prevent PHP Fatal Errors when setting navigation properties by adding proper union type declarations.

  • Adds union type declarations for navigationGroup and navigationIcon properties
  • Maintains backward compatibility with null defaults
  • Includes additional navigation property declarations for completeness

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.


// Fix for Filament v4 navigation property types
protected static string|\UnitEnum|null $navigationGroup = null;
protected static string|\BackedEnum|null $navigationIcon = null;
Copy link

Copilot AI Aug 31, 2025

Choose a reason for hiding this comment

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

The type declaration uses \BackedEnum but typically Filament v4 navigation icons accept string or classes implementing specific icon interfaces. Consider verifying if \BackedEnum is the correct type for navigationIcon or if it should be a more specific interface.

Suggested change
protected static string|\BackedEnum|null $navigationIcon = null;
protected static string|null $navigationIcon = null;

Copilot uses AI. Check for mistakes.
…ity with PHP enums

- removed the leading backslashes: string|UnitEnum|null and string|BackedEnum|null

- added proper imports
@ManukMinasyan
Copy link
Contributor

@yassersebai Hello, thank you for the PR. How can I reproduce that bug?

@yassersebai
Copy link
Author

Steps to Reproduce

  1. Create a new page extending BoardPage a long with navigation headers:
<?php

namespace App\Filament\Pages;

use Relaticle\Flowforge\BoardPage;
use Relaticle\Flowforge\Board;

class MyKanbanPage extends BoardPage
{
    protected static ?string $navigationIcon = 'heroicon-o-view-columns';
    protected static ?string $navigationLabel = 'My Kanban Board';
    protected static ?string $navigationGroup = "Operations";
    
    public function board(Board $board): Board
    {
        return $board
            ->query(MyModel::query())
            ->recordTitleAttribute('title')
            ->columnIdentifier('status')
            ->positionIdentifier('position')
            ->columns([
                Column::make('pending')->label('Pending'),
                Column::make('active')->label('Active'),
            ]);
    }
}
  1. Register the page in a Filament panel
  2. Access the application
  3. Fatal error occurs due to type mismatch

@ManukMinasyan
Copy link
Contributor

ManukMinasyan commented Aug 31, 2025

@yassersebai – For Filament V4, this is the correct way to configure navigation:
https://filamentphp.com/docs/4.x/navigation/overview#grouping-navigation-items

protected static string|BackedEnum|null $navigationIcon = null;
protected static string|UnitEnum|null $navigationGroup = null;
protected static ?string $navigationLabel = null;

No need to redeclare the same properties inside the board page. Please share your full error.

@yassersebai yassersebai closed this Sep 3, 2025
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