Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

[MM-22815][MM-23693][MM-23735] - Improve empty states on RHS search variants #5485

Merged
merged 29 commits into from
Jun 2, 2020

Conversation

nevyangelova
Copy link
Contributor

@nevyangelova nevyangelova commented May 13, 2020

Summary

As part of the UI quick wins, this PR handles empty states in the RHS search variants. Included is "mentions", "search", "flagged posts", "pinned posts" and "emoji picker". It add the "no_results_indicator.tsx" component adapted to match the respective designs.

Ticket Link

https://mattermost.atlassian.net/browse/MM-23693
https://mattermost.atlassian.net/browse/MM-22815
https://mattermost.atlassian.net/browse/MM-23735

@nevyangelova nevyangelova added 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester CherryPick/Approved Meant for the quality or patch release tracked in the milestone 1: UX Review Requires review by a UX Designer labels May 13, 2020
@nevyangelova nevyangelova added this to the v5.24.0 milestone May 13, 2020
Copy link
Contributor

@alifarooq0 alifarooq0 left a comment

Choose a reason for hiding this comment

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

Nice @nevyangelova! Just a question about FormattedHTMLMessage.

import NoResultsIndicator from 'components/no_results_indicator/no_results_indicator.tsx';
import FlagIcon from 'components/widgets/icons/flag_icon';

import {NoResultsVariant} from '../no_results_indicator/types';
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
import {NoResultsVariant} from '../no_results_indicator/types';
import {NoResultsVariant} from 'components/no_results_indicator/types';

width: 11px;
margin: 2px;
vertical-align: middle;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: empty line

components/search_results/search_results.jsx Outdated Show resolved Hide resolved
components/emoji_picker/emoji_picker.jsx Outdated Show resolved Hide resolved
@matthewbirtch matthewbirtch added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 13, 2020
@nevyangelova nevyangelova changed the title [MM-22815][MM-23693] - Improve empty states on RHS search variants [MM-22815][MM-23693][MM-23735] - Improve empty states on RHS search variants May 13, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 13, 2020
@matthewbirtch
Copy link
Contributor

@nevyangelova I tried spinning up a test server, but looks like it failed. I'll review when this is working again.

@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 13, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 13, 2020
@nevyangelova
Copy link
Contributor Author

/update-branch

@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 14, 2020
@nevyangelova
Copy link
Contributor Author

@matthewbirtch test server finally succeeded :D

@ogi-m ogi-m self-requested a review May 27, 2020 22:50
Copy link

@ogi-m ogi-m left a comment

Choose a reason for hiding this comment

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

@nevyangelova I'm seeing a few issues:

  1. Sidebar header overlaps the date separator:
    Screenshot (91)

  2. Mentions empty state not replaced with Search empty state:

  • Click on Recent Mentions (@ icon)
  • Enter something in search that won't return any results and hit enter
    Expected: Search empty state is shown
    Observed: Mentions empty state is shown
  1. Recent Mentions icon in active state after searching
  • Click on Recent Mentions (@ icon)
  • Enter something in search that will return results and hit enter
    Expected: Default/inactive icon is shown for Recent Mentions
    Observed: Active icon is shown for Recent Mentions
    Screenshot (92)

@nevyangelova
Copy link
Contributor Author

/update-branch

@jgilliam17
Copy link
Contributor

@nevyangelova
I checked for at-mentions missing hints when search term is cleared and see the hints missing - see gifs

  1. State: Without previous search history, user with no mentions
    i. Start by using cmd+shift+r before clicking on the @ icon
    ii. Click on @ followed by click on x to clear the search field < note no hint popover, field focused
    iii. Click outside of the search or click esc
    Observed: Search hint popup flashes and populates RHS
    no search history

  2. State: With previous search history, user with no mentions
    i. Search for a term with no results e.g apple, close RHS after searching and clear the field
    ii. Click on @, followed by x to clear the search field < note no hint popover, field focused
    iii. Click outside of the search field or click esc
    Observed: Search hint popup flashes briefly, replaced by the information for previous apple search
    Note that even if your apple search has previously returned some results, RHS still displays No results for "apple" at this point
    previous search history

@jgilliam17
Copy link
Contributor

@nevyangelova @matthewbirtch I noticed that empty states for mentions, pinned and flagged have added yet
Is this an intended change and acceptable?

@nevyangelova
Copy link
Contributor Author

@jgilliam17 this PR is approved by UX.

Copy link
Contributor

@jgilliam17 jgilliam17 left a comment

Choose a reason for hiding this comment

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

Thank you @nevyangelova

Tested, looks good to merge.

  • Verified emoji empty state
  • Verified empty state for mentions, flagged and pinned
  • Verified search empty state

Per discussion with Nevy, issue with search results displaying on the RHS after user clears the at-mention search can be addressed separately after merge and testing as it’s out of scope here.

@jgilliam17 jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Jun 2, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@nevyangelova nevyangelova merged commit f177e65 into master Jun 2, 2020
@nevyangelova nevyangelova deleted the MM-22815 branch June 2, 2020 15:02
@amyblais amyblais removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone 2: Dev Review Requires review by a core commiter labels Jun 2, 2020
@amyblais amyblais added this to the v5.26 milestone Jun 18, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation
Projects
None yet