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.

Sign up for GitHub

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

Jump to bottom

componentizied input-search and search in set currency and token management #9917

Merged
merged 1 commit into from
Feb 20, 2020
Merged

componentizied input-search and search in set currency and token management #9917

merged 1 commit into from
Feb 20, 2020

Conversation

tbenr
Copy link
Contributor

@tbenr tbenr commented Jan 24, 2020 •
edited
Loading

  • added search feature when selecting default currency
  • added search feature in asset management
  • moved search-input to a dedicated ui component
  • removed auto-correction and auto-capitalization in search input

image

image

note for testing: non-regression tests should be done on chat search:
image

status: ready

@tbenr tbenr requested a review from a team as a code owner January 24, 2020 22:36
@status-github-bot
Copy link

Pull Request Checklist

  • Docs: Updated the documentation, if affected
  • Docs: Added or updated inline comments explaining intention of the code
  • Tests: Ensured that all new UI elements have been assigned accessibility IDs
  • Tests: Signaled need for E2E tests with label, if applicable
  • Tests: Briefly described what was tested and what platforms were used
  • UI: In case of UI changes, ensured that UI matches Figma
  • UI: In case of UI changes, requested review from a Core UI designer
  • UI: In case of UI changes, included screenshots of implementation

@auto-assign auto-assign bot removed the request for review from a team January 24, 2020 22:36
@tbenr
Copy link
Contributor Author

tbenr commented Jan 24, 2020

@rachelhamlin to set default currency as per device settings we need to move from react-native-languages to react-native-localize dependency, which takes more time. For now I implemented better way to set it :)

@status-im-auto
Copy link
Member

status-im-auto commented Jan 24, 2020 •
edited
Loading

Jenkins Builds

Click to see older builds (15)
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
❌ 42a69e7 #1 2020-01-24 22:42:03 ~5 min android 📄log
❌ 42a69e7 #1 2020-01-24 22:42:03 ~5 min android-e2e 📄log
✔️ 42a69e7 #1 2020-01-24 22:46:14 ~9 min ios 📦ipa 📲
✔️ ed1d428 #2 2020-01-24 23:08:17 ~9 min ios 📦ipa 📲
✔️ ed1d428 #2 2020-01-24 23:16:35 ~18 min android-e2e 📦apk 📲
✔️ ed1d428 #2 2020-01-24 23:16:35 ~17 min android 📦apk 📲
✔️ 7583e29 #3 2020-01-26 13:52:35 ~9 min ios 📦ipa 📲
✔️ 7583e29 #3 2020-01-26 14:01:59 ~19 min android 📦apk 📲
✔️ 7583e29 #3 2020-01-26 14:01:59 ~19 min android-e2e 📦apk 📲
✔️ 092801c #4 2020-01-26 14:14:47 ~9 min ios 📦ipa 📲
✔️ 092801c #4 2020-01-26 14:23:00 ~17 min android-e2e 📦apk 📲
✔️ 092801c #4 2020-01-26 14:23:08 ~17 min android 📦apk 📲
✔️ 532efcc #5 2020-01-27 14:29:55 ~9 min ios 📦ipa 📲
✔️ 532efcc #5 2020-01-27 14:32:24 ~12 min android 📦apk 📲
✔️ 532efcc #5 2020-01-27 14:41:28 ~21 min android-e2e 📦apk 📲
❔ Commit #️⃣ Finished (UTC) Duration Platform Result
✔️ 9203db7 #6 2020-02-06 08:41:53 ~12 min ios 📦ipa 📲
✔️ 9203db7 #6 2020-02-06 08:50:35 ~21 min android-e2e 📦apk 📲
✔️ 9203db7 #6 2020-02-06 08:50:35 ~20 min android 📦apk 📲
✔️ 9203db7 #7 2020-02-17 12:32:05 ~9 min ios 📦ipa 📲
✔️ 9b52ea2 #8 2020-02-19 18:31:54 ~9 min ios 📦ipa 📲
✔️ 9b52ea2 #7 2020-02-19 18:48:02 ~27 min android-e2e 📦apk 📲
✔️ 9b52ea2 #7 2020-02-19 18:48:02 ~26 min android 📦apk 📲

@flexsurfer
Copy link
Member

thanks @tbenr ! that's great! do you think we could add search for assets management in this PR or in a separate one ?

@tbenr
Copy link
Contributor Author

tbenr commented Jan 26, 2020

@flexsurfer this morning I realize it was missing there too :) let me add it in this pr

@tbenr tbenr changed the title componentizied input-search and search in set currency componentizied input-search and search in set currency and token management Jan 26, 2020
@tbenr
Copy link
Contributor Author

tbenr commented Jan 26, 2020

@flexsurfer done!

tbenr
tbenr commented Jan 27, 2020
View reviewed changes
src/status_im/search/core.cljs Outdated Show resolved Hide resolved
@errorists
Copy link
Contributor

errorists commented Jan 27, 2020 •
edited
Loading

@tbenr thanks man! I wouldn't have it focused on entering the view tho. It's not critical but if possible I'd ask to change that

@tbenr
Copy link
Contributor Author

tbenr commented Jan 27, 2020

@errorists oky no prob!

@tbenr
Copy link
Contributor Author

tbenr commented Jan 27, 2020

@errorists done!

@weekly-digest weekly-digest bot mentioned this pull request Feb 2, 2020
Closed
@tbenr
Copy link
Contributor Author

tbenr commented Feb 6, 2020

rebased

@weekly-digest weekly-digest bot mentioned this pull request Feb 9, 2020
Closed
@churik churik self-assigned this Feb 17, 2020
@churik
Copy link
Member

churik commented Feb 17, 2020

Thank you for contribution and apologies for the delay in testing.
Noticed one small issue: value in the search field in asset or currency management is erased while switching tabs.
It is not reproducible with the search field in chat.
To reproduce type some value in search in wallet > settings > manage assets, switch tab and go back to wallet.
Expected, that value will be preserved.
In fact, value is empty, but search results are filtered according to the previous query.
manage-asset

@tbenr
Copy link
Contributor Author

tbenr commented Feb 18, 2020

@churik ok yes i recreated it. give me some time to fix it, i'm pretty busy these days.

@tbenr
Copy link
Contributor Author

tbenr commented Feb 19, 2020

@churik should be fixed now, I rebased too.

@churik
Copy link
Member

churik commented Feb 20, 2020

Retested all search fields on IOS13 and Android8.
Looks good to me!
Test case is added https://ethstatus.testrail.net/index.php?/cases/view/6269

@tbenr @churik
componentizied input-search and added in curency and token
81c4d16 
Signed-off-by: Churikova Tetiana <churikova.tm@gmail.com>
@churik churik merged commit 81c4d16 into status-im:develop Feb 20, 2020
@tbenr tbenr deleted the currency-search branch February 20, 2020 11:52
@weekly-digest weekly-digest bot mentioned this pull request Feb 23, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flexsurfer flexsurfer flexsurfer approved these changes

@errorists errorists errorists approved these changes

@Ferossgp Ferossgp Awaiting requested review from Ferossgp

@yenda yenda Awaiting requested review from yenda

Assignees

@churik churik

@tbenr tbenr

Labels
request-manual-qa Tested - OK
Projects
No open projects
Legacy QA Automation Project
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tbenr @status-im-auto @flexsurfer @errorists @churik