Base chat dedesign #13640
Base chat dedesign #13640
Conversation
briansztamfater
requested review from
cammellos,
flexsurfer,
Parveshdhull and
pavloburykh
Jenkins BuildsClick to see older builds (22)
|
90% of end-end tests have passedNot executed tests (3)Failed tests (18)Click to expand
Passed tests (165)Click to expand
|
67% of end-end tests have passedFailed tests (6)Click to expand
Passed tests (12)Click to expand
|
17% of end-end tests have passedFailed tests (5)Click to expand
Passed tests (1)Click to expand
|
|
@briansztamfater thank you for the fixes. ISSUE 1 White border around "pencil" element became wider than it used to be in original old UIexpected result: actual result: ISSUE 2 Color of online indicator changed its colorexpected result: actual result: |
|
ENS names do not show up in the new "Discord style" design. The user Dearest Powerful Sloth should show up as @bridge like it used to.
|
|
Sometimes the notification bar turns black when reopening a chat
|
Hey @du64 thanks for the issue, I couldn't reproduce it unfortunately, but if you can create an issue to track this separately would be better, as this PR is only modifying styling / UI and the issue may be related to something else.
|
|
@pavloburykh UI fixes you pointed out should be fixed, could you verify please? |
100% of end-end tests have passedPassed tests (2)Click to expand
|
@briansztamfater thank you for the fixes. Those issues are fixed. Now we will need to perform regression testing and run all critical and medium e2e before merging into develop. We will do it as soon as all final changes will be done and approved. Please, let me know when PR will be ready for final testing (you can just move it to E2E column when PR will pass the developers review). Thanx! |
| [react/text {:style (style/datemark-text)} | ||
| (string/capitalize value)]]]) |
There was a problem hiding this comment.
Hi, shouldn't we use quo2/text component which already has these text-styles defined
95% of end-end tests have passedFailed tests (4)Click to expand
Passed tests (80)Click to expand
|
50% of end-end tests have passedFailed tests (2)Click to expand
Passed tests (2)Click to expand
|
99% of end-end tests have passedNot executed tests (87)Failed tests (1)Click to expand
Passed tests (98)Click to expand
|
|
@briansztamfater please, take a look at following issue: ISSUE 3 Old UI Add (+) element on status screen has new UI style and is dislocatedActual result: Expected result: |
|
@briansztamfater also, could you please resolve existing conflicts here src/status_im/ui/screens/home/views.cljs ? Thanx! |
briansztamfater
force-pushed
the
feat/base-chat-redesign
branch
from
06d2512
to
6a4d561
Compare
|
@pavloburykh thanks for pointing out the issue, just rebased and pushed out the fix. Let me know if you find something else. |
92% of end-end tests have passedFailed tests (14)Click to expand
Passed tests (172)Click to expand
|
79% of end-end tests have passedFailed tests (3)Click to expand
Passed tests (11)Click to expand
|
|
@briansztamfater thank you for the fixes, PR is ready for merge. |
|
Hi @briansztamfater, Please also look into #13666 (comment) |
| :margin-horizontal 16 | ||
| :margin-top 15 | ||
| :margin-bottom 8} | ||
| [quo2.text/text {:size :heading-1 :weight :semi-bold} "Messages"] |
There was a problem hiding this comment.
Hard coding string will cause problems while supporting multiple languages
| :ellipsize-mode :middle | ||
| :weight :medium | ||
| :style {:color (quo2.colors/theme-colors quo2.colors/neutral-50 quo2.colors/neutral-40)}} | ||
| "Public"] |
There was a problem hiding this comment.
Same here, better to add in translations
| :style {:color (quo2.colors/theme-colors quo2.colors/neutral-50 quo2.colors/neutral-40)}} (i18n/label :t/members-count {:count (count contacts)})]] [quo2.text/text {:monospace true | ||
| :weight :medium | ||
| :style {:color (quo2.colors/theme-colors quo2.colors/neutral-50 quo2.colors/neutral-40)} | ||
| :number-of-lines 1 | ||
| :ellipsize-mode :middle} | ||
| (utils.utils/get-shortened-address chat-id)]))]]])) |
There was a problem hiding this comment.
When it is possible, please, keep lines' length shorter/equal 80 chars. (needs reformatting)
|
This comment, also needs to be addressed. |
Done! Let me know if we can merge |
Thank you, PR is ready to be merge |
Signed-off-by: Brian Sztamfater <brian@status.im>
briansztamfater
force-pushed
the
feat/base-chat-redesign
branch
from
24e0018
to
22f9267
Compare
Context: Reverted #13184 on #13638.
Now resubmitting chat redesign changes to track and fix some issues before merging
Issues: #13184 (comment)
status: ready