-
Notifications
You must be signed in to change notification settings - Fork 984
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
[#9657] system messages ui changes in group chat #9800
Conversation
Pull Request Checklist
|
Jenkins BuildsClick to see older builds (23)
|
mailserver error introduced not in this PR |
@flexsurfer totally, just stops me from testing in as I can't setup a group chat between a couple devices |
@ghmulti ok managed to test it, these look correct as in the design apart from the horizontal margin which should be 8px on both left and right sides, so it's even with chat bubbles. |
@errorists sure, there is a max-width limit defined for chat message view (red rectangle), should it be dropped for system messages then? here is view after removing max-width for system messages and adding margin 8px on both sides |
it's worth mentioning that changes related to "system messages" are made within generic chat message component, so style changes potentially may affect public groups and one-to-one chat views |
Hey @ghmulti, good work! I just have some suggestions, that may have made this type of message less dependent on the text message. What do you think about making them as a separate type in multi-method Thanks! |
Hello @ghmulti! |
@Ferossgp hey, thanks for feedback and I think you are right - had the same thought @churik sure, but I'll need to do some refactoring (#9800 (comment)) |
I meant when it will be ready. Thanks! |
for now - since there is no content-type-system-text defined in status-go, it is assigned manually based on status-message? flag in chat-message |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested latest commit, group chat system messages look as they should now, thanks!!
@cammellos done 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Serhy could you give it another quick round of testing? if this tests ok I can merge status-go and update the version with develop.
@ghmulti sorry for the significant delay. |
@cammellos could you please prepare new status-go build with content-type, assume it will not work with the old build after rebase |
@ghmulti sorry for the delay, I have been off for the past two weeks. To update status-go you can run:
and then you can restart the stack and it should pick up the new version. |
@churik hey, pushed updated version could you take a look please? |
94% of end-end tests have passed
Failed tests (6)Click to expand
Passed tests (88)Click to expand |
Tested ally types of group system messages, right order, fetching system messages from offline (Android 10, IOS 13). Thanks for a work @ghmulti ! |
Fixes #9657
Summary
Suggest to create separate task for joining together events without separation (chained events) as this pr affects mostly styling
Platforms
Tested on Android
Areas that maybe impacted
Group chats
status: wip (awaiting feedback)
Colored block for margin/padding overview