Skip to content
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

[WIP] feat: search content and users #375

Open
wants to merge 19 commits into
base: main
Choose a base branch
from

Conversation

awietz
Copy link
Contributor

@awietz awietz commented Jul 8, 2024

This pull request add the option to search for questions and answers, in addition to users.

The home "Users" tab has been renamed to "Search".

The search result now contains users, questions and answers, matching the entered search query, using a LIKE statement.

Questions and answers are only searched, when the search query is at least 3 characters long. Otherwise, only users are searched.

Only the first 10 users are returned in the search result (like before).

Only the first 10 questions and answers are returned in the search result.

When nothing is entered in the search field, a set of famous users are displayed instead (just like before).

search-result

QUESTIONS

  • Right now only the answer to a question is clickable as a link. Shouldn't it be the entire content element in the search results (question and answer)?
  • The answer to a question changes the background on hover. Should that be changed, in the search results, so the entire element changes background color instead, as it's also the entire element that is clickable?
  • Each question offers a lot of functions (such as like, etc.). Should these also work / we available in the arch results?

TODO

  • Sanitise search input (leading and trailing whitespace, ``` and other parsed characters...)
  • When query starts with @, only search for users (maybe later in another PR)
  • Layout for search results (mainly questions)
  • Tests

@nunomaduro
Copy link
Member

It's looking great; share some screenshots when you can.

Copy link
Collaborator

@CamKem CamKem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great so far!
I added a couple of suggestions you may like to look at.

resources/views/livewire/home/search.blade.php Outdated Show resolved Hide resolved
@awietz
Copy link
Contributor Author

awietz commented Jul 14, 2024

I have a few questions listed above, I would like to get feedback on.
I guess when they are answered, the feature can be finished and I can write tests covering all the changes.

@awietz awietz requested a review from CamKem July 14, 2024 13:53
Copy link
Collaborator

@CamKem CamKem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks really good. Just a name change of that blade file.

In terms of the styling, I'm not sure on the outline being present above the shared updates too... What do you guys think @MrPunyapal & @nunomaduro

Screenshot 2024-07-15 at 8 27 54 AM

Copy link
Collaborator

@CamKem CamKem Jul 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would name this component "user-card" - a more general name.

@awietz
Copy link
Contributor Author

awietz commented Jul 15, 2024

On the welcome page there is a users search field, teasing "Lots of interesting profiles".
That should probably remain unchanged, right?

Since the same component is used twice, there would be a little work left for me there.

@MrPunyapal
Copy link
Collaborator

As per Nuno I think default behaviour should be same and if someone searches it should bring Posts with users 🤔

@MrPunyapal MrPunyapal force-pushed the feat/search-content-and-users branch from 4339061 to 9005fae Compare November 29, 2024 12:45
@MrPunyapal MrPunyapal requested a review from CamKem November 29, 2024 13:16
@MrPunyapal
Copy link
Collaborator

@CamKem can you pull the latest and give some ideas on UI?

@CamKem CamKem mentioned this pull request Dec 20, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should called user-card.blade.php so it more clear what its purpose is.
The card in the file below should be move here.

Comment on lines +3 to +39
<a
href="{{ route('profile.show', ['username' => $user->username]) }}"
class="group flex items-center gap-3 rounded-2xl border border-slate-900 bg-slate-950 bg-opacity-80 p-4 transition-colors hover:bg-slate-900"
wire:navigate
>
<figure class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12 flex-shrink-0 overflow-hidden bg-slate-800 transition-opacity group-hover:opacity-90">
<img
class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12"
src="{{ $user->avatar_url }}"
alt="{{ $user->username }}"
/>
</figure>

<div class="flex flex-col overflow-hidden text-sm">
<div class="flex items-center space-x-2">
<p class="truncate font-medium">
{{ $user->name }}
</p>

@if ($user->is_verified && $user->is_company_verified)
<x-icons.verified-company
:color="$user->right_color"
class="size-4"
/>
@elseif ($user->is_verified)
<x-icons.verified
:color="$user->right_color"
class="size-4"
/>
@endif
</div>

<p class="truncate text-slate-500 transition-colors group-hover:text-slate-400">
{{ '@'.$user->username }}
</p>
</div>
</a>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<a
href="{{ route('profile.show', ['username' => $user->username]) }}"
class="group flex items-center gap-3 rounded-2xl border border-slate-900 bg-slate-950 bg-opacity-80 p-4 transition-colors hover:bg-slate-900"
wire:navigate
>
<figure class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12 flex-shrink-0 overflow-hidden bg-slate-800 transition-opacity group-hover:opacity-90">
<img
class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12"
src="{{ $user->avatar_url }}"
alt="{{ $user->username }}"
/>
</figure>
<div class="flex flex-col overflow-hidden text-sm">
<div class="flex items-center space-x-2">
<p class="truncate font-medium">
{{ $user->name }}
</p>
@if ($user->is_verified && $user->is_company_verified)
<x-icons.verified-company
:color="$user->right_color"
class="size-4"
/>
@elseif ($user->is_verified)
<x-icons.verified
:color="$user->right_color"
class="size-4"
/>
@endif
</div>
<p class="truncate text-slate-500 transition-colors group-hover:text-slate-400">
{{ '@'.$user->username }}
</p>
</div>
</a>
<div class="group flex items-center gap-3 rounded-2xl border dark:border-slate-900 border-slate-200 dark:bg-slate-950 bg-slate-50 dark:bg-opacity-80 p-4 transition-colors dark:hover:bg-slate-900 hover:bg-slate-100">
<figure class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12 flex-shrink-0 overflow-hidden bg-slate-800 transition-opacity group-hover:opacity-90">
<img
class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12"
src="{{ $user->avatar_url }}"
alt="{{ $user->username }}"
/>
</figure>
<div class="flex flex-col overflow-hidden text-sm text-left">
<a
class="flex items-center space-x-2"
href="{{ route('profile.show', ['username' => $user->username]) }}"
wire:navigate
x-ref="parentLink"
>
<p class="text-wrap truncate font-medium dark:text-white text-black">
{{ $user->name }}
</p>
@if ($user->is_verified && $user->is_company_verified)
<x-icons.verified-company
:color="$user->right_color"
class="size-4"
/>
@elseif ($user->is_verified)
<x-icons.verified
:color="$user->right_color"
class="size-4"
/>
@endif
</a>
<p class="truncate text-slate-500 transition-colors group-hover:text-slate-400">
{{ '@'.$user->username }}
</p>
</div>
<x-follow-button
:id="$user->id"
:isFollower="auth()->check() && $user->is_follower"
:isFollowing="auth()->check() && $user->is_following"
class="ml-auto"
wire:key="follow-button-{{ $user->id }}"
/>
</div>

Comment on lines +34 to +83
@php($user = $result)
<li
data-parent=true
x-data="clickHandler"
x-on:click="handleNavigation($event)"
wire:key="user-{{ $user->id }}"
>
<div class="group flex items-center gap-3 rounded-2xl border dark:border-slate-900 border-slate-200 dark:bg-slate-950 bg-slate-50 dark:bg-opacity-80 p-4 transition-colors dark:hover:bg-slate-900 hover:bg-slate-100">
<figure class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12 flex-shrink-0 overflow-hidden bg-slate-800 transition-opacity group-hover:opacity-90">
<img
class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12"
src="{{ $user->avatar_url }}"
alt="{{ $user->username }}"
/>
</figure>
<div class="flex flex-col overflow-hidden text-sm text-left">
<a
class="flex items-center space-x-2"
href="{{ route('profile.show', ['username' => $user->username]) }}"
wire:navigate
x-ref="parentLink"
>
<p class="text-wrap truncate font-medium dark:text-white text-black">
{{ $user->name }}
</p>

@if ($user->is_verified && $user->is_company_verified)
<x-icons.verified-company
:color="$user->right_color"
class="size-4"
/>
@elseif ($user->is_verified)
<x-icons.verified
:color="$user->right_color"
class="size-4"
/>
@endif
</a>
<p class="truncate text-slate-500 transition-colors group-hover:text-slate-400">
{{ '@'.$user->username }}
</p>
</div>
<x-follow-button
:id="$user->id"
:isFollower="auth()->check() && $user->is_follower"
:isFollowing="auth()->check() && $user->is_following"
class="ml-auto"
wire:key="follow-button-{{ $user->id }}"
/>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After changing the name & adding the markup for the user card component, utilise it here.

Suggested change
@php($user = $result)
<li
data-parent=true
x-data="clickHandler"
x-on:click="handleNavigation($event)"
wire:key="user-{{ $user->id }}"
>
<div class="group flex items-center gap-3 rounded-2xl border dark:border-slate-900 border-slate-200 dark:bg-slate-950 bg-slate-50 dark:bg-opacity-80 p-4 transition-colors dark:hover:bg-slate-900 hover:bg-slate-100">
<figure class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12 flex-shrink-0 overflow-hidden bg-slate-800 transition-opacity group-hover:opacity-90">
<img
class="{{ $user->is_company_verified ? 'rounded-md' : 'rounded-full' }} h-12 w-12"
src="{{ $user->avatar_url }}"
alt="{{ $user->username }}"
/>
</figure>
<div class="flex flex-col overflow-hidden text-sm text-left">
<a
class="flex items-center space-x-2"
href="{{ route('profile.show', ['username' => $user->username]) }}"
wire:navigate
x-ref="parentLink"
>
<p class="text-wrap truncate font-medium dark:text-white text-black">
{{ $user->name }}
</p>
@if ($user->is_verified && $user->is_company_verified)
<x-icons.verified-company
:color="$user->right_color"
class="size-4"
/>
@elseif ($user->is_verified)
<x-icons.verified
:color="$user->right_color"
class="size-4"
/>
@endif
</a>
<p class="truncate text-slate-500 transition-colors group-hover:text-slate-400">
{{ '@'.$user->username }}
</p>
</div>
<x-follow-button
:id="$user->id"
:isFollower="auth()->check() && $user->is_follower"
:isFollowing="auth()->check() && $user->is_following"
class="ml-auto"
wire:key="follow-button-{{ $user->id }}"
/>
</div>
<li>
<x-user-card
data-parent=true
x-data="clickHandler"
x-on:click="handleNavigation($event)"
wire:key="user-{{ $result->id }}"
:user="$result"
/>
</li>

@CamKem
Copy link
Collaborator

CamKem commented Dec 20, 2024

@CamKem can you pull the latest and give some ideas on UI?

@MrPunyapal - Pulled the latest (sorry for the delay) and had a look, it might be a good idea to include @nunowar regarding UI considerations, as this will work in with the new design, where the search bar will be the one at the top in #733 implementation.

I would say once my review has been addressed we could merge this for now as an interim solution, then we can update the cards to fit with the new design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants