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.

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

[FIX #2118] ui: store search-key under [:toolbar-search :show] #2585

Merged
merged 1 commit into from
Dec 27, 2017

Conversation

vitvly
Copy link
Contributor

@vitvly vitvly commented Dec 2, 2017

Addresses #2118.

Summary:

Chat list used to store booleans under [:toolbar-search :show] which is
different from other views and caused the toolbar to appear in Chats if search was used in Discover or Contacts. With this change search key (= :chat-list) is stored under the above path.

Review notes (optional):

Testing notes (optional):

Steps to test:

  • Open Status
  • Go to Discover tab
  • Click on Search icon in the toolbar and type some text
  • Go to Chats tab
  • Actual result: no chats visible
  • Expected result: should see the list of recent chats. Search filter from another tab should not be applied.

status: ready

@oskarth
Copy link
Contributor

oskarth commented Dec 6, 2017

Same comment as #2584 (comment) - sorry to be a stickler but it really makes it easier (and quicker!) for devs and QA to review, test and merge your changes.

@churik churik self-assigned this Dec 14, 2017
@@ -77,13 +77,13 @@
(defview chats-list []
(letsubs [chats [:filtered-chats]
edit? [:get-in [:chat-list-ui-props :edit?]]
search? [:get-in [:toolbar-search :show]]
Copy link
Contributor

Choose a reason for hiding this comment

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

Use of undeclared Var status-im.ui.screens.chats-list.views/search? at line 98 /Users/jenkins/workspace/status-react/parametrized build/src/status_im/ui/screens/chats_list/views.cljs

search? symbol is used later in this function

Copy link
Contributor

@rasom rasom left a comment

Choose a reason for hiding this comment

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

see comment above

@churik churik removed their assignment Dec 14, 2017
@vitvly vitvly force-pushed the bug/empty-chats-#2118 branch from 462b563 to daae98e Compare December 14, 2017 10:52
@vitvly
Copy link
Contributor Author

vitvly commented Dec 14, 2017

Sorry, lame mistake. Fixed.

Chat list used to store booleans under [:toolbar-search :show] which is
different from other views and caused the issue described in status-im#2118.
@rasom rasom force-pushed the bug/empty-chats-#2118 branch from daae98e to 907d3c7 Compare December 22, 2017 10:44
@rasom
Copy link
Contributor

rasom commented Dec 22, 2017

i've just rebased it onto develop

@churik churik self-assigned this Dec 22, 2017
@churik
Copy link
Member

churik commented Dec 22, 2017

Checked:
install:
Chats:

@flexsurfer flexsurfer added the bug label Dec 27, 2017
@flexsurfer flexsurfer merged commit 9f15a2d into status-im:develop Dec 27, 2017
qoqobolo added a commit that referenced this pull request May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants