mattermost-webapp
mattermost-webapp copied to clipboard
[MM-36280] Show recent searches in Search popover (#18841)
Summary
Show recent searches of the user is the search bar.
QA
- Make a search query
- Reopen the search bar
- Select a search type: file or message
- Make sure that the recent searches are shown in the list
- Make sure that the list is navigable with the mouse also the keyboard (up and down arrows)
- Make sure that the behavoir of the recent search items matches the description in Figma
Ticket Link
Related Pull Requests
- This PR in the server side could be done seperately and once it's merged we can add the remove button (https://github.com/mattermost/mattermost-server/pull/21148)
Screenshots
before | after |
---|---|
![]() |
![]() |
Release Note
Show 5 most recent searches in the search popover
Hello @goulinkh,
Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.
E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.
Creating a new SpinWick test server using Mattermost Cloud.
Mattermost test server created! :tada:
Access here: https://mattermost-webapp-pr-11244.test.mattermost.cloud
Account Type | Username | Password |
---|---|---|
Admin | sysadmin | Sys@dmin123 |
User | user-1 | User-1@123 |
Thanks @hmhealey and @nevyangelova for the detailed review and explanation especially for the scroll behavior :slightly_smiling_face: I've fixed all the proposed changes feel free to give it another go!
New commit detected. SpinWick will upgrade if the updated docker image is available.
Mattermost test server updated with git commit 4bfbc94a0321ffaf2bdde37e9da4a36c62ad1568
.
Access here: https://mattermost-webapp-pr-11244.test.mattermost.cloud
This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!
@hmhealey Thanks for the review! I've addressed your comments, please let me know if there is anything else to change!
This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!
LG, just update the failing specs:)
LG, just update the failing specs:)
@nevyangelova I couldn't figure out the reason of the test failure, sorry!
/update-branch
Error trying to update the PR. Please do it manually.
please update with latest master from our repo and rerun again :)
@esethna Note that deleting recent searches isn't possible yet on this test server because the corresponding server PR hasn't been merged yet, but we can test the rest of the functionality
@goulinkh It looks like the failing test is an actual failure and not something that was fixed by merging the latest changes in. Would you mind taking a look at that?
@goulinkh It looks like the failing test is an actual failure and not something that was fixed by merging the latest changes in. Would you mind taking a look at that?
Should be good to go!
This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!
Forwarding to QA review @jgilliam17. Feel free to ping me if needed
/e2e-test
/update-branch
/e2e-test
Successfully triggered E2E testing! GitLab pipeline | Test dashboard
/update-branch
/e2e-test
Successfully triggered E2E testing! GitLab pipeline | Test dashboard
Thanks @goulinkh Really looking forward to having recent search history Seeing few small issues
- "Recent Searches" font style and opacity doesn't match Figma @asaadmahmood can you please provide direction
![Screen Shot 2023-01-24 at 2 17 08 PM](https://user-images.githubusercontent.com/52937121/214387806-8e9980aa-f0f7-478b-b3f3-d58bd62e64e3.png)
- "Recent searches" is not vertically centered, the whole header seem to be a bit larger than it needs to be. There is a lot of white space.
Apologies I accidentally clicked on close while adding my comment
Continued:
- When searching for Files, we are not displaying "Recent Files". Let me know if this is supposed to be included in this PR.
- When selecting one of the recents searches, new search entry was created. I expected search term to be only moved up on the list based on recency, instead new item is created e.g. search for "pdf", select "pdf" from the list of recent searches observed: new "pdf" search is added to the list
- Clicking on
x
to clear the search, only removes label, Messages or Files, and leaves search term. We should clear the whole field.
https://user-images.githubusercontent.com/52937121/214395259-1cb4eb7d-53ae-49a3-946c-98aa21b123fd.mov
E2E report looks good, no PR related failures. @goulinkh Would you be open to adding a Cypress test for the search history?
This PR has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!
/update-branch
Error trying to update the PR. Please do it manually.