Add hide/show for limitList #9228
Conversation
|
This is good default behaviour, please also replicate it for TextEntry if that is appropriate |
|
Nice one, Hugh! |
@zepfietje weirdly, if I switch the div to an x-filament::link, it destroys the list formatting. Any ideas? EDIT - nevermind, if I use tag="button" it renders properly. 2023-10-21_20-46-28.mp4 |
|
Yeah the link defaults to be a real link, so it needs the |
|
I think it should be gray |
|
Yeah I prefer that as well. |
packages/infolists/resources/views/components/text-entry.blade.php
Outdated
Show resolved
Hide resolved
packages/infolists/resources/views/components/text-entry.blade.php
Outdated
Show resolved
Hide resolved
|
I've updated the link color to gray for all instances in this PR. |
|
OK, changed showLimited to isLimited in both table and infolist. Tweaked docs for infolist limitList() to match table. Tested the Infolist stuff in the demo app. I think that's everything? 2023-10-23_15-48-57.mp4 |
|
Not sure if we need that |
@zepfietje hmm, I actually don't like it without, it's TOO snappy, especially if you have a lot of hidden items, and suddenly the everything just snaps off the bottom of the page. That's why I added it in the first place. But you are The Man, so tell me to remove it and I will. But at least test it with 50+ hidden items. |
|
Yeah I do understand your reasoning, @cheesegrits. |
| 'less_list_items' => 'Show :count less', | ||
| 'more_list_items' => 'Show :count more', |
There was a problem hiding this comment.
I think we should/could add this functionality to checkbox lists as well.
I'm mentioning this now since that'd make it easier to extract the translations instead of doing that later.
Thoughts?
There was a problem hiding this comment.
Checkbox lists don't have limits at the moment, and I don't think this needs transferring yet
| > | ||
| {{ trans_choice('filament-tables::table.columns.text.more_list_items', $limitedArrayStateCount) }} | ||
| <x-filament::link |
There was a problem hiding this comment.
I feel like adding an icon (e.g. + or -) would make it more obvious that the link is a link after a long list of text. Not too sure yet though. What do you think?
There was a problem hiding this comment.
I'll try it and see.
There was a problem hiding this comment.
The font weight is already enough IMO, as well as the underline on hover. It is pretty clear that it's not just another list item.
packages/infolists/resources/views/components/text-entry.blade.php
Outdated
Show resolved
Hide resolved
| @@ -193,8 +203,24 @@ class="cursor-pointer max-w-max" | |||
| @if ($limitedArrayStateCount = count($limitedArrayState ?? [])) | |||
| <{{ $isListWithLineBreaks ? 'li' : 'div' }} | |||
| class="text-sm text-gray-500 dark:text-gray-400" | |||
| x-on:click.prevent="isLimited = ! isLimited" | |||
There was a problem hiding this comment.
We don't need these attributes on this wrapper anymore, right?
There was a problem hiding this comment.
Which attributes? The classes?
There was a problem hiding this comment.
And the click handler. I guess these changes are from before we moved to using links?
danharrin
added
enhancement
|
Decided this should be opt in, as people with huge limited lists could experience performance issues with the extra DOM elements |
As requested by my users ... when using limitList() for listWithLineBreaks() in a TextColumn, replaces "and X more" text with "show X more" clickable text which reveals the additional values, and a "show X less" to hide them again.
Lmk if you want this behavior but would would prefer this behavior to be optional, perhaps with a named argument like limitList(5, hideShow: true).
2023-10-20_21-02-08.mp4