Status-react: In profile address is shown instead of name and "Mailserver" label is overlapped when connected to custom mailserver

Created on 9 Sep 2019  路  8Comments  路  Source: status-im/status-react

Description

Type: Bug

Summary: when connected to custom mailserver, address is shown instead of name and no "Mailserver" label in Profile > Sync settings

Expected behavior

photo_2019-09-09 14 18 00

Actual behavior

dfs

Reproduction

  • Open Status
  • Go to profile > Sync settings
  • Add custom mailserver
  • Connect to it
  • Go back

Solution

Summary:
I'm actually not sure how we can handle mailservers with long names.
For now "Mailserver" label is overlapped, which is not right IMO:

photo_2019-09-09 14 26 34

@andmironov or @errorists please take a look, what is better from your POV.

Additional Information

  • Status version: nightly09/09/2019
  • Operating System: Android, iOS
bug

Most helpful comment

thanks for commenting @bitsikka I appreciate your in-depth thoughts on this! I like what you're suggesting even if it doesn't match the semantic use of defined props to achieve this goal :) I don't want us to split hair over such a detail, the important bit is to truncate the accessory in the middle (since this way there's less chance the truncation will make recognising the selected choice among multiple others difficult) and block the text accessory from pushing pass this 62% threshold you suggest, so it doesn't force overflow on titles like on the attached screenshots.

All 8 comments

The Mailserver text label should not get truncated or overlapped, no question there 馃憤

This is the desired behaviour, 16px of blank space between and truncate the text accessory (mail server address) in the middle
Small Cell

so the following would work only on iOS but React Native does support text prop like adjustsFontSizeToFit where it will try and shrink the text down to make it fit up to a minimum scale factor minimumFontScale where 0.7 sounds like a reasonable value. No such luck on Android :(

This is how it would look like. It's optional, if it's only two lines of code to add we might as well do it.
Small Cell

so the following would work only on iOS but React Native does support text prop like adjustsFontSizeToFit

I like it, but is it going to truncate the value if it's extremely long anyway?

Here's how this edge case is specified in our design system at the moment

Screen Shot 2019-09-09 at 16 58 19

interesting! :)

I handeled it differently in #8912 so far

  • I made it so that text accessory is no longer than 62% of device width. To be precise, I did this
(defn accessory-text [width last]
  {:color       colors/gray
   :max-width   (if last (* @width 0.62) (* @width 0.55))
   :line-height 22})

In that way, I kept it simple, for now.

Rationale is that in the nesting structure in the implementation, it is made such that column containing title (includes title-prefix, title-accessory, subtitle, subtitle accessory) is supposed to be the primary content - the king/queen if you will. Two columns on its side(icon/accessories) are supposed to be secondary/accessorial/peripheral if you will. So, title has flex 1 and is supposed to eat up majority of space, which it does on most cases.

A long accessory taking over the king/queen :) is in that perspective, a misbehavior :)

What I propose is, if there is a long content which wants to be aligned towards accssories, we put it in title. The black title goes in title-prefix. This facility exists already. We even have title-color-override to make it gray instead of default black. What would be needed is conditional 16px left margin on title, and title alignment prop. That would achieve the intent of the spec.

Also, title-elipsize-mode, because title has tail by default.

Note: list-item as it is implemented does NOT use any absolute positioning, negative margin, only one case of transform translateY(for chat-list unread count badge), flex-grow, flex-shrink and flex-basis. In my opinion, it is best to stay away from those as much as possible.

thanks for commenting @bitsikka I appreciate your in-depth thoughts on this! I like what you're suggesting even if it doesn't match the semantic use of defined props to achieve this goal :) I don't want us to split hair over such a detail, the important bit is to truncate the accessory in the middle (since this way there's less chance the truncation will make recognising the selected choice among multiple others difficult) and block the text accessory from pushing pass this 62% threshold you suggest, so it doesn't force overflow on titles like on the attached screenshots.

Another possibility is, without any change:

  • make black title (eg Mailserver) a title-prefix
  • put the long text (eg looooong.looong.mail.server.name) in title as a component(yes, title can be component in this implementatino)
  • in the title component use text view with :margin-left 16, text-align right, ellipsize-mode middle

That ought to do it.. no change necessory in the implementation of list-item

I did (as in the last idea) this(in #8912 ) for

  • node-version and app-version in Profile > About >
  • Mailserver in Profile > Sync Settings >
[list-item/list-item
     {:type                :small
      :accessibility-label :node-version
      :title-prefix        :t/node-version
      :title
      [react/text
       {:number-of-lines 1
        :ellipsize-mode  :middle
        :style
        {:color        colors/gray
         :padding-left 16
         :text-align   :right
         :line-height  22}}
       node-version]}]

Not: About this issue's Original Post.. the part about

address is shown instead of name

is not handled in #8912. I figure mailserver-subscription need to be fixed, and that is not a part of the PR focused on polishing/putting-finishing-touches of list-item

Still relevant in nightly 07/01/2020

This problem also occurs in the about section with status version.

IMG_BF83F39D27F2-1

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jakubgs picture jakubgs  路  44Comments

jakubgs picture jakubgs  路  174Comments

hesterbruikman picture hesterbruikman  路  41Comments

jeluard picture jeluard  路  45Comments

flexsurfer picture flexsurfer  路  45Comments