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

[MM-25373] [MM-25374] [MM-25106] - Fix functionality bugs in search hint popover #5610

Merged
merged 9 commits into from
Jun 2, 2020

Conversation

nevyangelova
Copy link
Contributor

Summary

This PR addresses functionality bugs related to the search hint popover. I have tested thoroughly but let me know if you find anything else.

Ticket Link

https://mattermost.atlassian.net/browse/MM-25373
https://mattermost.atlassian.net/browse/MM-25374
https://mattermost.atlassian.net/browse/MM-25106#### Related Pull Requests

@nevyangelova nevyangelova added 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 labels May 27, 2020
@nevyangelova nevyangelova added this to the v5.24.0 milestone May 27, 2020
@jgilliam17 jgilliam17 added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 28, 2020
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 28, 2020
@nevyangelova nevyangelova added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label May 28, 2020
Copy link
Contributor

@sudheerDev sudheerDev left a comment

Choose a reason for hiding this comment

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

LGTM

visibleSearchHintOptions = searchHintOptions.filter((option) => {
return new RegExp(pretext, 'ig').test(option.searchTerm) && option.searchTerm.toLowerCase() !== pretext.toLowerCase();
});
} catch (err) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is err needed? Didn't linter complain?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops. Will remove

@jgilliam17
Copy link
Contributor

@nevyangelova
"type in "in: " (include the space character at the end)"
Popup no longer overlap on the 1st space, but if you accidentally add 2nd space it overlaps again

@nevyangelova
Copy link
Contributor Author

@jgilliam17 fixed

@jgilliam17
Copy link
Contributor

Thank you @nevyangelova
Looks good, one issue remains, see 3. below

  1. MM-25373 - Verified search hint popover does not display over autocomplete list including the instance where uses adds extra spaces
  2. MM-25374 - Verified search inputs are not added inadvertently while continuing to hover over area where hints were previously displayed and hitting Enter
  3. MM-25106 - Verified search hint popover was present whenever user focused on the field except in one instance > After clicking @ mentions icon and deleting text input using x or backspace

@nevyangelova
Copy link
Contributor Author

@jgilliam17 thanks, Ogi found this issue in #5485 and its fixed there. Sorry for the mash-up.

@jgilliam17
Copy link
Contributor

@nevyangelova
Thank you for the more info on @ mention issue
Apologize, I missed this on my last pass, I was looking mainly at linked issues

  • Search Hints are no longer auto sorting as user types
    Search auto-sort

  • When search hints are displayed on the RHS, focus is not placed in the text box as user selects from the list of hints
    RHS search focus

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

  • Verified search hint popover does not display over autocomplete list
  • Verified search inputs are not added inadvertently while hovering and hitting Enter
  • Verified search hint popover was present when user focused on the search field.
  • Verified auto-sorting as user types is working again and dash size has been increased for visibility
    Issues that were raised with at-mentions and when hint list is part of the RHS will be tested after merge as it's addressed in the different PR.

@jgilliam17 jgilliam17 removed the 3: QA Review Requires review by a QA tester label Jun 2, 2020
@jgilliam17 jgilliam17 added 4: Reviews Complete All reviewers have approved the pull request and removed 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 292018d into master Jun 2, 2020
@nevyangelova nevyangelova deleted the fix-functional-bugs-popover branch June 2, 2020 12:52
@mattermod mattermod added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Jun 2, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation and removed 2: Dev Review Requires review by a core commiter labels Jun 2, 2020
mattermod pushed a commit that referenced this pull request Jun 2, 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 CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants