set width boundaries for long usernames in activity center #13185
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (18)
|
flexsurfer
requested changes
Mar 21, 2022
| {:margin-left 72 | ||
| :margin-top 12 | ||
| :margin-right 50}) | ||
| (let [title-text-width (* @(re-frame/subscribe [:dimensions/window-width]) 0.65)] |
There was a problem hiding this comment.
its better to pass width as a parameter from the view
There was a problem hiding this comment.
I agree!
working on this
There was a problem hiding this comment.
@flexsurfer : done
flexsurfer
approved these changes
Mar 21, 2022
| @@ -1,6 +1,7 @@ | |||
| (ns status-im.ui.screens.notifications-center.styles | |||
| (:require [quo.design-system.colors :as colors] | |||
| [quo.design-system.spacing :as spacing])) | |||
| [quo.design-system.spacing :as spacing] | |||
| [re-frame.core :as re-frame])) | |||
There was a problem hiding this comment.
fixing and squashing asap :)
There was a problem hiding this comment.
removed unused import
|
@siddarthkay thanx for the PR. Please, take a look at this issue ISSUE 1New usernames and messages are highlighted in activity center. STEPS:
Actual result:
OS: Android/IOS
ISSUE 2Please, take a look at lint errors in last builds |
|
@pavloburykh : Um yeah my bad, I left in the background highlighting( which i use internally for debugging ) , I'm pushing a fix for that |
siddarthkay
force-pushed
the
bug/13178-long-username-is-overlaid-on-timestamp-in-activity-center
branch
from
98ac9c4
to
11fd3b3
Compare
|
@pavloburykh : fix pushed, should be good for QA now. |
95% of end-end tests have passedFailed tests (4)Click to expand
Passed tests (75)Click to expand
|
96% of end-end tests have passedFailed tests (3)Click to expand
Passed tests (76)Click to expand
|
|
@siddarthkay thanx for fixes! ISSUE 3It seems that on IOS (iPhone X) long username is still located to close to timestamp. That's definitely a minor but if this is an easy fix lets improve it. Below 2 screenshots for comparison from IOS (iPhone X) and Android (Samsung Galaxy A52) with a message from same user iPhone x Samsung Galaxy A52 |
briansztamfater
approved these changes
Mar 22, 2022
|
@pavloburykh : I'm working on reproducing this in iphoneX Simulator and then will push a fix |
@siddarthkay thank you! let me know if you need any help on reproduction. |
|
Latest fix should take care of this case @pavloburykh |
86% of end-end tests have passedFailed tests (11)Click to expand
Passed tests (68)Click to expand |
95% of end-end tests have passedFailed tests (4)Click to expand
Passed tests (75)Click to expand
|
✔️ status-react/prs/ios/PR-13185#8 🔹 ~12 min 🔹 8529e18 🔹 📦 ios package |
|
@siddarthkay thanx for fixes, ready for merge. Sorry for testing delay, was busy by release testing. |
siddarthkay
force-pushed
the
bug/13178-long-username-is-overlaid-on-timestamp-in-activity-center
branch
from
8529e18
to
8aaf37a
Compare
lint fix remove unused import and debug backgrounds set width boundaries for long usernames in activity fix lint and pass width from view updated spacing to accommodate iphoneX Signed-off-by: andrey <motor4ik@gmail.com>
flexsurfer
force-pushed
the
bug/13178-long-username-is-overlaid-on-timestamp-in-activity-center
branch
from
8aaf37a
to
c40094f
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[comment]: Fixes #13178
Platforms
Android
iOS
status: ready