-
Notifications
You must be signed in to change notification settings - Fork 0
Adding the user filtering/search #17
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…-base into user_search * 'main' of https://github.com/CornellCustomDev/cd-laravel-base: adding the app sass file spliting out the flux elements and renaming them
woodseowl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to run this locally and got JS errors on the search box. Is it working for you on your local?
inaydich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I downloaded the code, but the search is not working for me: nothing happens
| </button> | ||
| </div> | ||
| </flux:field> | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we use our csd components here (label, input, button)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to use csd components. Check it out again
| public function sort($column) { | ||
|
|
||
| if ($this->sortBy === $column) { | ||
| if ($this->sortBy === $column) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it an extra space added here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was misaligned.
inaydich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good and works. Could you try to replace the flux components with our CSD components?
Yep, just replaced it. |
woodseowl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<x-cds.input
label="Filter by name"
placeholder="Search users..."
class="w-full"
wire:model.live="nameFilter"
clearable
/>
I'd love to see the filter be done this way. It takes advantage of Livewire and Flux, removing the needs for all of these things:
- reset button (with "clearable"),
- 'filterByName()' and 'resetFilter()' functions (because of "model.live")
- form and the submit button (because of "model.live")
- and flux:field is redunant
I think the only thing we'd need to address is an appropriate aria-live=polite attribute.
Maybe we could have this after this filter:
<div aria-live="polite" class="sr-only">
@if ($this->users->total() > 0)
<div>Showing {{ $this->users->firstItem() }} to {{ $this->users->lastItem() }} of {{ $this->users->total() }} results
</div>
@else
<div>No results found</div>
@endif
</div>
No description provided.