Skip to content

Conversation

@aitorvs
Copy link
Collaborator

@aitorvs aitorvs commented Feb 12, 2021

Task/Issue URL: https://app.asana.com/0/414730916066338/1199931182139250/f
Tech Design URL:
CC:

Description:
When we fixed the requery pixels (#1065) we also had to tune the search bar behavior, causing a side effect where URLs in thee search bar would never be selected when the user taps on the search bar.

This PR fixes that issue

Steps to test this PR:

  1. install/update from this branch

  2. Ensure tests in https://app.asana.com/0/0/1199922227966104/f pass (to see when rq pixels are sent, filter by rq_ in logcat)

  3. install/update from this branch

  4. open app and tap on search bar

  5. search for "reddit"

  6. select "🔎 reddit"

  7. tap on the search bar

  8. verify text is not selected and autocomplete results show up

  9. open the first URL in the SERP

  10. tap on the search bar

  11. verify URL text is selected


Internal references:

Software Engineering Expectations
Technical Design Template

@marcosholgado marcosholgado self-requested a review February 12, 2021 17:01
@marcosholgado marcosholgado self-assigned this Feb 12, 2021
Copy link
Contributor

@marcosholgado marcosholgado left a comment

Choose a reason for hiding this comment

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

All rq test passing but there is one thing that I've noticed.

After selecting a URL (didFocusAlready = false) and going back to SERP, the whole search term is selected and autocomplete is not triggered. I think if we remove didFocusAlready from the if in the line 49 it should be fixed. I think is enough with text?.isWebUrl() == false since the previous focus state should not matter anymore.

  1. Search for lego.
  2. Click on any search result.
  3. Click the URL, the whole url gets selected as expected.
  4. Using the back button go back to SERP.
  5. Select the url. The full search term is selected rather than autocomplete being triggered.

@aitorvs
Copy link
Collaborator Author

aitorvs commented Feb 15, 2021

All rq test passing but there is one thing that I've noticed.

After selecting a URL (didFocusAlready = false) and going back to SERP, the whole search team is selected and autocomplete is not triggered. I think if we remove didFocusAlready from the if in the line 49 it should be fixed. I think is enough with text?.isWebUrl() == false since the previous focus state should not matter anymore.

  1. Search for lego.
  2. Click on any search result.
  3. Click the URL, the whole url gets selected as expected.
  4. Using the back button go back to SERP.
  5. Select the url. The full search term is selected rather than autocomplete being triggered.

Thanks @marcosholgado it makes sense, done!

Copy link
Contributor

@marcosholgado marcosholgado left a comment

Choose a reason for hiding this comment

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

Nice job! 🚢 🇮🇹

@aitorvs aitorvs merged commit 8f46aa7 into develop Feb 15, 2021
@aitorvs aitorvs deleted the fix/aitor/search_bar branch February 15, 2021 14:14
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.

2 participants