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

fix(searchBox) - Conversation search UX improvements #9997

Merged
merged 4 commits into from
Jul 31, 2023

Conversation

DorraJaouad
Copy link
Contributor

@DorraJaouad DorraJaouad commented Jul 17, 2023

☑️ Resolves

  • Fix Conversation search UX improvements #9974

  • The new conversation '+' button is now hidden when the search field is expanded

  • The abort search button is visible whenever the search box is focused.

  • The list gets immediately cleared out from filters once the search field is clicked on.

  • If the search results are empty, a new button shows up to quickly create a conversation with the search text as a name

  • The animation is more expedited ( set to 0.15s instead of 0.3s) as it requires to be quick.

Recording.2023-07-28.143911.mp4

Other

And if the search results are empty, you can create a new conversation with the search text, it will be set to private by default.
image

screenshots

Before
image

After
image

🚧 Tasks

  • Code review

🏁 Checklist

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Very nice, thanks for the fix :)

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Actually, there is still a bug that happens if we try and create a new conversation during a search operation.

Screen.Recording.2023-07-18.at.14.46.08.mov

I think that the best solution here is to remove the + button too during a search operation. There's really no need to keep it there and it's easy enough to interrupt the search given the presence of the x button even if there's no text in the search-box

@DorraJaouad
Copy link
Contributor Author

I also agree, having the '+' button available during the search experience can be thought that it has to do sthg with it.

@nickvergessen
Copy link
Member

nickvergessen commented Jul 18, 2023

I think that the best solution here is to remove the + button too during a search operation. There's really no need to keep it there and it's easy enough to interrupt the search given the presence of the x button even if there's no text in the search-box

I always search for things and when I can not find the room have to create it. Would be annoying if I have to clear the search first, before being able to create it (in the past we added code to use the search term as default value for the name input on the dialog, in case you remember)

But maybe adding a placeholder/option in the search list could be a replacement?

@nickvergessen nickvergessen added this to the 💜 Next Major (28) milestone Jul 18, 2023
@marcoambrosini
Copy link
Member

But maybe adding a placeholder/option in the search list could be a replacement?

Definitely, let's create a NcListItem if there are no matches that creates a new conversation without asking anything about participants and settings. Just the name and no participants like in old talk?

@DorraJaouad
Copy link
Contributor Author

But maybe adding a placeholder/option in the search list could be a replacement?

Definitely, let's create a NcListItem if there are no matches that creates a new conversation without asking anything about participants and settings. Just the name and no participants like in old talk?

something like this ?
image

Or we can make a placeholder right below the search box that contains a link and says something like :
"No conversations found. Consider initiating a new conversation."

But I think we should just open the dialog and the search text is pasted in the Name field instead of directly creating one.

@nickvergessen
Copy link
Member

Just the name and no participants like in old talk?

Or we bring up the dialog on page one with the name prefilled. I'd prefer that one.

@Antreesy
Copy link
Contributor

Antreesy commented Jul 19, 2023

Or we bring up the dialog on page one with the name prefilled

Should be easy to realise with showModalForItem(), like we're doing it now for groups / circles

P.S. I'd wait with this PR until open conversation feature will be merged, as it'll be relatively eaiser to resolve conflicts from here

@marcoambrosini
Copy link
Member

marcoambrosini commented Jul 20, 2023

Or we bring up the dialog on page one with the name prefilled. I'd prefer that one.

I think we should skip the dialog for this one and allow a very quick way to get a conversation going.
If a conversation is new and it des not have participants other than you we can display a couple of buttons right in the chat to make public and copy link in one action and add participants in another one.

That'd be for the future, but for now I think it would be really valuable to have a very quick way to create a conv

@DorraJaouad let's use the ListItem component with the chatPlus icon.

Screenshot 2023-07-20 at 14 18 07

Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Haven't tested it yet, sorry

src/components/LeftSidebar/LeftSidebar.vue Outdated Show resolved Hide resolved
src/components/LeftSidebar/LeftSidebar.vue Outdated Show resolved Hide resolved
src/components/LeftSidebar/SearchBox/SearchBox.vue Outdated Show resolved Hide resolved
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Nice :)

One last thing:
Upon creating a new conversation via the list-item, the search operation should end and the route should change to that conversation


edit. found another one:

Screenshot 2023-07-21 at 12 59 46

If you're not searching and filtering and the filtering has no results, the create conversation shortcut shouldn't be there. Also the empty-content shouldn't say "no search result", but something more like "No unread messages" or "No unread mentions"

@Antreesy
Copy link
Contributor

Also the empty-content shouldn't say "no search result"

We could go with common wording for all cases "No matches found", like in this PR:
https://github.com/nextcloud/spreed/pull/9955/files#diff-7a1abf98ca549db81a418814a97629c796bc85dc32b40f9cffb65ce3ad40d555R125

@DorraJaouad DorraJaouad force-pushed the fix/9974/imporve_search_UX branch 2 times, most recently from 8da3ffb to 88149a3 Compare July 21, 2023 14:28
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Apart from weird blur-focus behavior, looks good

@DorraJaouad DorraJaouad marked this pull request as draft July 24, 2023 17:24
@DorraJaouad DorraJaouad marked this pull request as ready for review July 25, 2023 10:04
Copy link
Contributor

@Antreesy Antreesy left a comment

Choose a reason for hiding this comment

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

Works on web client, looks also cool!

src/components/LeftSidebar/LeftSidebar.vue Show resolved Hide resolved
@DorraJaouad DorraJaouad force-pushed the fix/9974/imporve_search_UX branch 2 times, most recently from 814a89f to a6f05dd Compare July 25, 2023 19:43
<NcActionButton close-after-click
@click="showModalListConversations">
<!-- Actions -->
<div v-show="!isFocused"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe put them into one <div> wrapper with pos: absolute and display: flex, as they are following the same logic?
then <transition-group> should be replaces with <transition>, and i think, it should work without key param

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that leads to a different animation as it will be counted on the wrapper and not on each button but let's wait for @marcoambrosini to check whether to keep it or not.

animation2023-07-28.114440.mp4

Copy link
Member

Choose a reason for hiding this comment

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

Looks good, as discussed we've increased the animation speed

DorraJaouad and others added 4 commits July 28, 2023 15:38
Signed-off-by: DorraJaouad <[email protected]>
Signed-off-by: Maksim Sukharev <[email protected]>
Signed-off-by: DorraJaouad <[email protected]>
@DorraJaouad DorraJaouad merged commit 92bdbab into master Jul 31, 2023
18 checks passed
@DorraJaouad DorraJaouad deleted the fix/9974/imporve_search_UX branch July 31, 2023 09:02
@nextcloud nextcloud deleted a comment from backportbot-nextcloud bot Aug 3, 2023
@DorraJaouad
Copy link
Contributor Author

/backport to stable27

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

Successfully merging this pull request may close these issues.

Conversation search UX improvements
4 participants