mokhosh / filament-kanban

Add kanban boards to your Filament pages
https://filamentphp.com/plugins/mokhosh-kanban
MIT License
228 stars 32 forks source link

[Bug]: Status as integer (correction) #15

Closed envolute closed 5 months ago

envolute commented 5 months ago

What happened?

Hello Mokhosh!!

Congratulations on the work, really good and simple to implement in the project.

I have a Model where the "Status" field is an integer. I use an Enum (int) to define the values. Even so, I configured it and, initially, everything worked normally. However, I identified and fixed some problems:

1º - Status Title: I saw that it shows the status title based on the Enum value, as in my case it is an integer, it shows the ID. Correction: Observing the "IsKanbanStatus" trait, I saw that it was enough to implement the getTitle() method passing the titles, which shows the Status correctly... See the code below...

enum statusEnum:int implements HasLabel
{
    use IsKanbanStatus;
    case TODO = 0;
    case DOING = 1;
    case DONE = 2;
    public function getLabel(): ?string {
        return match ($this) {
            self::TODO => 'To-Do',
            self::DOING => 'In Progress',
            self::DONE => 'Done',
        };
    }
    public function getTitle(): ?string {
        return $this->getLabel();
    }
}

2º - Conflict between Status and Record When I tried to drag an item to another status, I saw that I could drag internal elements from Records separately. I was able to drag a div from a record into another record or to another status and, obviously, this generated an error... I noticed that the problem is that the code identifies the "Status" container by the tag ID, and the tag ID is generated from the status value, which in my case, is an integer. This generates a <div id=1> conflict, as the record tag ID is also loaded from the record ID in the database So, depending on the value of the item, we can have 2 elements with the same id. See the image below... Desktop-screenshot Of course this problem will not occur with items with larger IDs. Ex: id="185466", But if you test with few items, it may occur...

Correction: As the objective is to load the status ID from a string, so as not to conflict with the record ID, I added a prefix to the status ID tag.

[Final Step-by-step]

File [pages/KanbanBoard.php #37] Add prefix variable: protected static string $statusPrefix = 'status'; So I can change the prefix if necessary

File: [kanban-status.blade.php #7] I changed from: id="{{ $status['id'] }}" for id="{{ self::$statusPrefix }}-{{ $status['id'] }}"

File: [kanban-scripts.blade.php #32] I changed from: const statuses = @js($statuses->map(fn ($status) => $status['id'])) for const statuses = @js($statuses->map(fn ($status) => self::$statusPrefix.'-'.$status['id']))

Then, for the system to pick up the correct Status, I just remove the prefix where necessary

File: [concerns/HasStatusChange.php #12 / #30] Add this line: $status = str_replace($statusPrefix.'-', '', $status);

public function statusChanged(int $recordId, string $status, array $fromOrderedIds, array $toOrderedIds): void
{
         $status = str_replace($statusPrefix.'-', '', $status);
         $this->onStatusChanged($recordId, $status, $fromOrderedIds, $toOrderedIds);
}
public function sortChanged(int $recordId, string $status, array $orderedIds): void
{
        $status = str_replace($statusPrefix.'-', '', $status);
        $this->onSortChanged($recordId, $status, $orderedIds);
}

Desktop-screenshot (1)

How to reproduce the bug

Use an Enum (int) to define the Status values

Package Version

2.3.0

PHP Version

8.1.10

Laravel Version

10.10

Which operating systems does with happen with?

Windows

Which browsers does with happen with?

Chrome

Notes

No response

envolute commented 5 months ago

An observation: Before making adjustments to the code I saw that in addition to the problem of "dragging" elements, statuses with larger values did not allow moving items... I believe this problem may be related to issue #12 If so, this fix would also resolve this...

envolute commented 5 months ago

An adjustment: In the code above there is an error related to the file [concerns/HasStatusChange.php #Lines: 12 / 30] Wrong: $status = str_replace($statusPrefix.'-', '', $status); Right: $status = str_replace(self::$statusPrefix.'-', '', $status);

mokhosh commented 5 months ago

Thanks! I appreciate the detailed issue.

As for 1º, I had talked about this in the videos, but now I've added it to the docs so it's more clear how you can customize the title.

For 2º, I fixed it in a different way that doesn't require you to choose a prefix, add it as a salt in the font end, and then remove it in the backend.

mokhosh commented 5 months ago

Check https://github.com/mokhosh/filament-kanban/releases/tag/v2.4.0

envolute commented 5 months ago

Great, mokhosh!! I updated and everything is actually working perfectly...

Thanks!!