Add select component #52
Conversation
|
I don't have time to look at the PR atm but hopefully soon. In the meantime can you rebase and solve the merge conflicts? |
|
it's done |
|
TL;DR: Let's optimise this component for elequent collections instead of arrays, since you'll likely work much more with eloquent collections in a typical Laravel app. Right now, this component accepts simple arrays. The keys will be used as values in the html, the values in the array will be used as label in the html. Imagine you want to create a select element with let's say all departments in your app. Departments are stored in the db. In your controller you might retrieve them like this. Department::all()In order to pass the models to the view component you'd have to perform a $departments = Department::all()->mapWithKey(function(Department $department) {
return [$department->id => $department->name]
});Having to do this will grow old pretty fast. I propose to change this component so that it accepts an This way you don't need to do any transformation and this would just work. <x-select :values="$departments" />Optionally, This is just a rough sketch, I hope you get the idea. Alternatively a second Blade component could be introduced (could be named |
|
@driesvints can we merge this? It sounds like a miss on the package :/ |
|
@LonnyX sorry I haven't gotten to this yet. I've been a bit swamped the last couple of weeks so I'm running behind on issues/prs on Blade UI Kit. I'm hoping to get through them somewhere soon although it's hard to say when exactly. I also want to think out this particularly PR a bit better before merging like Freek suggests. |
|
@Ufooo , if i make it as multiple selection, i won't be able to set selected values. |
|
@freekmurze did you think so? :) |
|
@Ufooo Nice! |
|
when will this PR be merged? |
|
I think we need to make some edits, @Ufooo are you willing to still maintain this PR or would you like me to fork and make the changes on my own? |
| id="{{ $id }}" | ||
| {{ $attributes }} | ||
| > | ||
| @if(!$slot->isEmpty()) |
There was a problem hiding this comment.
| @if(!$slot->isEmpty()) | |
| @if(! $slot->isEmpty()) |
| $this->placeholder = $placeholder; | ||
|
|
||
| if ($options instanceof Collection) { | ||
| $this->options = $options->mapWithKeys(function ($option) { |
There was a problem hiding this comment.
We can refactor this mapWithKeys() expression to use pluck()
| $this->options = $options->mapWithKeys(function ($option) { | |
| $this->options = $options->pluck($this->nameAttribute, $this->valueAttribute) |
| return in_array($value, $selected); | ||
| } | ||
|
|
||
| return ($value === $selected); |
There was a problem hiding this comment.
| return ($value === $selected); | |
| return $value === $selected; |
|
|
||
| public function isSelected($value): bool | ||
| { | ||
| $selected = old($this->name, $this->selected); |
There was a problem hiding this comment.
What is the purpose of the old() call here?
| /** @var string */ | ||
| public $placeholder; | ||
|
|
||
| public function __construct(string $name, $options, string $placeholder = '', string $nameAttribute = 'name', string $valueAttribute = 'id', $id = null, $selected = null) |
There was a problem hiding this comment.
I think that a more appropriate name for $nameAttribute is probably $labelAttribute - each option has a (display) label and value. Thoughts?
|
Until this gets merged laravel-form-components has a complete selection of form components. |
well, am gonna leave a footprint for this PR almost 20 months after. |
No description provided.