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

Fix key uniqueness in message history and chat list #4578

Merged
merged 9 commits into from
Jul 2, 2018
Merged

Fix key uniqueness in message history and chat list #4578

merged 9 commits into from
Jul 2, 2018

Conversation

vitvly
Copy link
Contributor

@vitvly vitvly commented May 31, 2018 •
edited
Loading

Fix #4225, #4226

Also correct typo in Figwheel output-to file name for desktop build.

@vitvly vitvly requested a review from vkjr May 31, 2018 19:07
@vitvly vitvly requested review from rcullito, vkjr and maxhora and removed request for vkjr May 31, 2018 19:07
rcullito
rcullito approved these changes May 31, 2018
View reviewed changes
Copy link
Contributor

@rcullito rcullito left a comment

Choose a reason for hiding this comment

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

Looks good!

@churik churik added the desktop label Jun 1, 2018
@vitvly vitvly changed the title Fix key uniqueness in message history [WIP] Fix key uniqueness in message history Jun 1, 2018
@maxhora
Copy link
Contributor

maxhora commented Jun 12, 2018

status-im/react-native-desktop-qt#252 should resolve ui glitches with messages history

@vitvly vitvly changed the title [WIP] Fix key uniqueness in message history Fix key uniqueness in message history and chat list Jun 13, 2018
@vkjr
Copy link
Contributor

vkjr commented Jun 16, 2018

@siphiuel, should this be merged?

@vitvly
Copy link
Contributor Author

vitvly commented Jun 18, 2018

Needs to be tested first.

@churik
Copy link
Member

churik commented Jun 18, 2018 •
edited
Loading

1. Empty space above text input field

Steps:

  1. log in
  2. open chat with 10+ message

Actual result:

monosnap 2018-06-18 15-59-05

Expected result:

no space between last message and text input field

Note: is not reproducible on other PRs
Build: PR-4578, 8

@churik
Copy link
Member

churik commented Jun 18, 2018

2. Last message can be overlapped by text field

Steps:

  1. log in
  2. join any public chat
  3. paste long issue (in my case 497 chars)
  4. send it

Actual result:

123456

Expected result:

123

Note: is not reproducible on other PRs
Build: PR-4578, 8

@churik
Copy link
Member

churik commented Jun 18, 2018 •
edited
Loading

Important note:

full testing is blocked by #4730

@churik churik mentioned this pull request Jun 19, 2018
Closed
@vitvly vitvly force-pushed the fix/4226-mixed-message-history branch from 370cf13 to d7088eb Compare June 25, 2018 14:15
@vitvly
Copy link
Contributor Author

vitvly commented Jun 25, 2018 •
edited by ghost
Loading

Decreased the bottom padding to chat list to match design (https://zpl.io/bWY4LKx).

Branch has been merged with desktop, so updated chat window does not seem to exhibit the problem #2.

Zeplin
Collaboration app for UI designers and frontend developers

@churik
Copy link
Member

churik commented Jun 28, 2018 •
edited
Loading

1 is fixed.
2 is reproduced if you send several long messages in a row (i.e. 3 one-by-one). Is not reproducible on current Desktop branch.

But I've experienced one more problem:

3. Message history is overlapping when you have several chats with history>1 day (when switching between chats)

Steps:
Requirements: have 1-1 and public chat with history more than 1 day

  1. log in
  2. switch between chats with history>1 day (i.e. "Today" and "Yesterday")
  3. check history in 1-1 ha

Actual result:

"Today" disappears
messages from "Yesterday" are overlapping between chats
m1

Expected result:

no overlapping

NOTE: if you switch to chat with "Today" messages only, and then to chat with several days history - you cannot see an issue until you switch to another chat with several days history.

@vitvly
Copy link
Contributor Author

vitvly commented Jun 28, 2018

After conversation with @Maxris it seems that #2 and #3 are related to status-im/react-native-desktop-qt#253.

@churik
Copy link
Member

churik commented Jun 29, 2018 •
edited
Loading

Issue with chat history is moved to #4995

@vitvly vitvly merged commit dfa2557 into desktop Jul 2, 2018
This was referenced Jul 5, 2018
Closed
Closed
@rasom rasom deleted the fix/4226-mixed-message-history branch September 3, 2018 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vkjr vkjr vkjr approved these changes

@maxhora maxhora maxhora approved these changes

@yenda yenda yenda approved these changes

@rcullito rcullito rcullito approved these changes

Assignees

@vitvly vitvly

Labels
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.
7 participants
@vitvly @maxhora @vkjr @churik @rcullito @yenda @jakubgs