fix: text column placeholder #7439

ashleyhood · 2023-08-03T22:45:12Z

This solves #7401

Changes have been thoroughly tested to not break existing functionality.

This solves filamentphp#7401

what-the-diff · 2023-08-03T22:45:25Z

PR Summary

Array Update in text-entry.blade.php and text-column.blade.php
The variable $arrayState was updated in text-entry.blade.php and text-column.blade.php files. The purpose is to always ensure this variable is an array. This provides more stability to the program so you won't face issues or errors when it isn't in an expected format.
Method Update in CanFormatState.php
The formatState() method in the CanFormatState.php file has been revised. Instead of using evaluate($this->placeholder), it now uses getPlaceholder(). This change enhances the reliability of the program as it allows us to better control the state format, potentially reducing error scenarios.

danharrin · 2023-08-03T23:49:31Z

Hey, where does $placeholder come from here? Have you tested this?

ashleyhood · 2023-08-04T00:00:25Z

I have tested this on my local dev machine.

The $placeholder comes from:

filament/packages/tables/src/Columns/Concerns/CanFormatState.php

Line 32 in 3e8455b

protected string | Closure | null $placeholder = null;

This is why it is wrapped in filled() to make sure it only does this if a placeholder is set and the state is null.

sumardi · 2023-08-04T02:44:50Z

The issue is about TextEntry. You should add this in text-entry.blade.php as well. And can we prepend null instead of empty string "". It is better if we want to format state, for example

->formatStateUsing(fn ($state) => $state?->format('d F Y'))

danharrin · 2023-08-04T09:40:44Z

@ashleyhood I'm sorry I don't understand, a protected property should not be accessible in the view like that. Are you on the latest version?

ashleyhood · 2023-08-04T10:14:26Z

I'm on Filament v3.0.7 and Livewire v3.0.0-beta.7.

That is really weird, because it does read the $placeholder and only changes the $arrayStates if it is filled!?!

How would I track down where this is being set if it is not in the CanFormatState trait?

Would be helpful to have livewire/wiretap 😜

ashleyhood · 2023-08-04T10:22:51Z

On second thoughts, isn't this okay to access the $placeholder property because it is being set in the blade @php directive and therefore being rendered before sending the request?

ashleyhood · 2023-08-04T10:42:00Z

My bad 😕

This is not working as it is returning:

Filament\Tables\Columns\TextColumn::placeholder(Closure|string|null $placeholder): static

Which will always be true.

We could only check for blank($arrayState) or we make the $placeholder accessible from a getter

public function getPlaceholder(): string | Closure | null
{
    return $this->placeholder;
}

and then check if it is `filled`.

ashleyhood · 2023-08-04T12:13:09Z

This will also need to be add to the InfoLists text-entry.blade.php as mentioned by @sumardi above.

Should that be in a separate PR or could it be added to this PR?

danharrin · 2023-08-06T20:29:54Z

Yeah we can make a getPlaceholder(). And please make the changes to infolists here instead of a new PR.

danharrin · 2023-08-06T23:03:37Z

packages/infolists/resources/views/components/text-entry.blade.php

@@ -37,6 +37,10 @@
        }

        $arrayState = \Illuminate\Support\Arr::wrap($arrayState);
+
+        if (blank($arrayState) && filled($getPlaceholder())) {
+            $arrayState = \Illuminate\Support\Arr::prepend($arrayState, '');


Doesn't this need to prepend the actual placeholder as state instead of an empty string? Also, can't it just reassign $arrayState completely into a new array since its already blank, instead of prepending

This needs to be an empty string because when we call formatState () on line 69 it expects a blank() state:

filament/packages/tables/src/Columns/Concerns/CanFormatState.php

Line 226 in 64811c8

if (blank($state)) {

You are right about just assigning a new array and that makes sense.

Lets use null instead of an empty string please

Potentially, we don't want the placeholder to run through formatState() though 🤔 It might need different styling, for example. Maybe we should use an if statement in the view to decide whether or not to render a placeholder. And then remove the placeholder application from the formatState() method

I think that looking at how the placeholder is formatted might be for another PR?

This PR was about trying to address how a placeholder works as intended only if the state is an empty string but not if the state is null.

ashleyhood · 2023-08-08T09:20:01Z

I have removed the Arr::wrap that was evaluating null values as an empty array. This keeps the same results for an empty string and allows a null value to give same results as an empty string.

ashleyhood · 2023-08-08T12:30:01Z

This also brings up the fundamental difference between $defaultState and $placeholder as there seems to be an overlap. For instance, if the state is null and both $defaultState and $placeholder are set then $defaultState will be used but if the state is an empty string then $placeholder is used.
Why would an empty string be replaced by the $placeholder? Wouldn't the expectation be the same as the $defaultState?

danharrin · 2023-08-10T07:54:06Z

$placeholder should probably have different styling and is not simply a "formatting" thing, whereas $defaultState acts as if it came from the database

ashleyhood · 2023-08-10T08:45:17Z

$placeholder should probably have different styling and is not simply a "formatting" thing, whereas $defaultState acts as if it came from the database

I see your point. Would you be happy to merge this fix and then another PR could address the issue of having a placeholder that accepted it's own styling.

DanielSpravtsev · 2023-08-25T09:53:54Z

Any news about state of this PR? Maybe help needed? @danharrin

danharrin · 2023-08-27T11:05:15Z

Work needs to be done on it, but by the time I have time to write instructions I might as well do it myself

danharrin · 2023-09-09T22:08:02Z

Alright this should be fixed up now for all columns and entries, they have a differently-styled placeholder

faizananwerali · 2023-09-12T13:15:27Z

Not working now for me in Infotlist with relationship. It was working before. The last working version was v3.0.42, after that, it has not been fixed. tried every single version to find the last working version.

TextEntry::make('profile.father_name')
    ->label('Father Name')
    ->placeholder('N/A'),
TextEntry::make('profile.father_occupation')
    ->label('Father Occupation/Profession')
    ->placeholder('N/A'),
TextEntry::make('profile.grand_father_name')
    ->label('Grand Father Name')
    ->placeholder('N/A'),
TextEntry::make('profile.mother_name')
    ->label("Mother's Name")
    ->placeholder('N/A'),
TextEntry::make('profile.mother_from')
    ->label("Mother's Home Town")
    ->placeholder('N/A'),
TextEntry::make('profile.mother_occupation')
    ->label("Mother's Occupation/Profession")
    ->placeholder('N/A'),
TextEntry::make('profile.guardian_phone_number')
    ->label('Guardian Phone Number')
    ->placeholder('N/A'),
TextEntry::make('profile.brothers')
    ->label('No. of Brothers')
    ->placeholder('N/A'),
TextEntry::make('profile.sisters')
    ->label('No. of Sisters')
    ->placeholder('N/A'),
TextEntry::make('profile.brothers_married')
    ->label('No. of Brothers Married')
    ->placeholder('N/A'),
TextEntry::make('profile.sisters_married')
    ->label('No. of Sisters Married')
    ->placeholder('N/A'),

This is how it looks.

Right now I using formatStateUsing to resolve this temporarily.

TextEntry::make('profile.father_name')
    ->label('Father Name')
    ->formatStateUsing(fn (User $record): string => $record->profile->father_name ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.father_occupation')
    ->label('Father Occupation/Profession')
    ->formatStateUsing(fn (User $record): string => $record->profile->father_occupation ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.grand_father_name')
    ->label('Grand Father Name')
    ->formatStateUsing(fn (User $record): string => $record->profile->grand_father_name ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.mother_name')
    ->label("Mother's Name")
    ->formatStateUsing(fn (User $record): string => $record->profile->mother_name ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.mother_from')
    ->label("Mother's Home Town")
    ->formatStateUsing(fn (User $record): string => $record->profile->mother_from ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.mother_occupation')
    ->label("Mother's Occupation/Profession")
    ->formatStateUsing(fn (User $record): string => $record->profile->mother_occupation ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.guardian_phone_number')
    ->label('Guardian Phone Number')
    ->formatStateUsing(fn (User $record): string => $record->profile->guardian_phone_number ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.brothers')
    ->label('No. of Brothers')
    ->formatStateUsing(fn (User $record): string => $record->profile->brothers ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.sisters')
    ->label('No. of Sisters')
    ->formatStateUsing(fn (User $record): string => $record->profile->sisters ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.brothers_married')
    ->label('No. of Brothers Married')
    ->formatStateUsing(fn (User $record): string => $record->profile->brothers_married ?? 'N/A')
    ->placeholder('N/A'),
TextEntry::make('profile.sisters_married')
    ->label('No. of Sisters Married')
    ->formatStateUsing(fn (User $record): string => $record->profile->sisters_married ?? 'N/A')
    ->placeholder('N/A'),

This is how now it looks.

$this->record is the User Model. I've set the record this way.

   public function infolist(Infolist $infolist): Infolist
   {
       return $infolist
           ->record($this->record)
           ->schema([
               Section::make(__('Family Info'))
                   ->heading('')
                   ->schema([
                       Grid::make(3)
                           ->schema([
                               // ....
                           ])
                   ])
       ]);
   }

danharrin · 2023-09-13T11:27:25Z

Please open an issue as I cannot reproduce it

fix: text column placeholder

cbdd08b

This solves filamentphp#7401

zepfietje marked this pull request as draft August 4, 2023 09:56

zepfietje added the bug Something isn't working label Aug 4, 2023

zepfietje added this to the v3 milestone Aug 4, 2023

ashleyhood added 2 commits August 6, 2023 23:28

Merge branch 'filamentphp:3.x' into fix_text_column_placeholder

883bafb

add placeholder getter

2a8e9af

ashleyhood force-pushed the fix_text_column_placeholder branch from 2a9a825 to 2a8e9af Compare August 6, 2023 22:55

Merge branch 'filamentphp:3.x' into fix_text_column_placeholder

786e3f1

ashleyhood marked this pull request as ready for review August 6, 2023 22:57

danharrin reviewed Aug 6, 2023

View reviewed changes

ashleyhood added 2 commits August 8, 2023 10:13

Merge branch 'filamentphp:3.x' into fix_text_column_placeholder

3eb6929

removed Arr::wrap() from view to allow null state

43fabaf

This was referenced Aug 29, 2023

Fix placeholder for Infolist TextEntry #8080

Closed

Fix placeholder for Tables TextColumn #8081

Closed

danharrin added 6 commits September 9, 2023 22:57

Merge branch '3.x' into pr/7439

c06405a

wip

30b7df8

Update text-column.blade.php

adfbea6

Update repeatable-entry.blade.php

3904d12

Update text-entry.blade.php

b9b0b75

Update text-column.blade.php

156d2f2

wip

fcbfce5

danharrin linked an issue Sep 9, 2023 that may be closed by this pull request

placeholder() not working on Relationship / TextColumn::make('relationship.name') #7823

Closed

danharrin merged commit ae2edc8 into filamentphp:3.x Sep 9, 2023
4 checks passed

danharrin mentioned this pull request Sep 9, 2023

placeholder() not working on Relationship / TextColumn::make('relationship.name') #7823

Closed

harvey-4thd mentioned this pull request Jan 31, 2024

TextEntry placeholder not showing for related Model attributes #11189

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: text column placeholder #7439

fix: text column placeholder #7439

ashleyhood commented Aug 3, 2023

what-the-diff bot commented Aug 3, 2023 •

edited

danharrin commented Aug 3, 2023

ashleyhood commented Aug 4, 2023

sumardi commented Aug 4, 2023 •

edited

danharrin commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

danharrin commented Aug 6, 2023

danharrin Aug 6, 2023

ashleyhood Aug 7, 2023

danharrin Aug 7, 2023

danharrin Aug 7, 2023

ashleyhood Aug 8, 2023

ashleyhood commented Aug 8, 2023

ashleyhood commented Aug 8, 2023

danharrin commented Aug 10, 2023

ashleyhood commented Aug 10, 2023

DanielSpravtsev commented Aug 25, 2023

danharrin commented Aug 27, 2023

danharrin commented Sep 9, 2023

faizananwerali commented Sep 12, 2023 •

edited

danharrin commented Sep 13, 2023

fix: text column placeholder #7439

fix: text column placeholder #7439

Conversation

ashleyhood commented Aug 3, 2023

what-the-diff bot commented Aug 3, 2023 • edited

PR Summary

danharrin commented Aug 3, 2023

ashleyhood commented Aug 4, 2023

sumardi commented Aug 4, 2023 • edited

danharrin commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

ashleyhood commented Aug 4, 2023

danharrin commented Aug 6, 2023

danharrin Aug 6, 2023

Choose a reason for hiding this comment

ashleyhood Aug 7, 2023

Choose a reason for hiding this comment

danharrin Aug 7, 2023

Choose a reason for hiding this comment

danharrin Aug 7, 2023

Choose a reason for hiding this comment

ashleyhood Aug 8, 2023

Choose a reason for hiding this comment

ashleyhood commented Aug 8, 2023

ashleyhood commented Aug 8, 2023

danharrin commented Aug 10, 2023

ashleyhood commented Aug 10, 2023

DanielSpravtsev commented Aug 25, 2023

danharrin commented Aug 27, 2023

danharrin commented Sep 9, 2023

faizananwerali commented Sep 12, 2023 • edited

danharrin commented Sep 13, 2023

what-the-diff bot commented Aug 3, 2023 •

edited

sumardi commented Aug 4, 2023 •

edited

faizananwerali commented Sep 12, 2023 •

edited