Skip to content
This repository has been archived by the owner on Feb 25, 2020. It is now read-only.

[WIP] fix: set default numberOfLines prop to 1 in HeaderTitle #294

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pzatorski
Copy link
Member

Description

I added back default numberOfLines prop to the HeaderTitle Component. It will truncate the default title text. [#290]
This PR would not fix the above issue, because HeaderTitle still overlaps on the headerLeft and/or headerRight.
IMG_0308

Next steps

I'm not sure how you would like to handle positioning in the Header Component if text is long (overlaps).

@pzatorski pzatorski changed the title fix: set default numberOfLines prop to 1 in HeaderTitle [WIP] fix: set default numberOfLines prop to 1 in HeaderTitle Nov 18, 2019
@pzatorski pzatorski requested a review from satya164 November 18, 2019 09:03
@satya164
Copy link
Member

we know the back button layout, so it'd be possible to set a horizontal margin of the same size.

though imo truncating the title is bad. since title is for users to recognize what the screen is. truncating it prevents it. instead developers should ensure not to put anything that's too long in the title.

@tanoabeleyra
Copy link

@satya164 I'm using the header bar as a search bar, the user can write whatever he want and it will be displayed as the title, how should I handle this situation where the input isn't a fixed/known string?

I don't think it's bad to truncate it in this scenario, the user would know what the full title is anyway because he wrote it. Besides that, a text that overlaps with the buttons or, even worse, overflows the screen won't be readable anyway. I would prefer it to be truncated and just see the dots.

By the way, if there's an input in the header it should hide the first letters to avoid overlapping, take WhatsApp or Telegram search as example.

@satya164
Copy link
Member

satya164 commented Jan 6, 2020

overflows the screen won't be readable anyway. I would prefer it to be truncated and just see the dots

overflow is worse, but that doesn't mean that truncating isn't bad. design guidelines such as material guidelines specifically advise against it. if you know, title can be long, it's always possible to come up with a different design (e.g. large header which shrinks on scroll, title inside the screen etc.).

regardless, handling this properly isn't even possible because of async APIs of React Native regarding layout measurement. you're always going to see a flicker before we can truncate it. it's not going to change until Fabric releases.

if there's an input in the header it should hide the first letters to avoid overlapping

there are a lot of limitations regarding what can be done. it's impossible to automatically detect of there is an input or not. however, there are props we provide such as headerBackTitleVisible which you can use to hide the back label when you know you have an input. you can also provide a horizontal margin in your headerTitle and customize it to truncate it yourself. it's not possible to do it automatically and provide a good user experience with the current limitations of React Native.

@tanoabeleyra
Copy link

Thanks for the fast response @satya164.

it's impossible to automatically detect of there is an input or not

I didn't mean that it should detect if there's an input, I was describing how it should behave in that situation.

The problem is that all the elements of the header have an absolute position and no width (or max width), that's why they overlap.

If I set a width to the title then it behave as what I consider is correct:

  • Doesn't overlap, it truncates with ellipsis.
  • The input shows only the text that fills in the width but you can navigate with a swipe to display previous/next characters.

I don't really like to play with the width, even less when there's no calc() in RN.

@satya164
Copy link
Member

satya164 commented Jan 7, 2020

The problem is that all the elements of the header have an absolute position and no width (or max width), that's why they overlap.

They are using absolute position because it's impossible to achieve the desired layout using flexbox. We cannot set a width or max width for the same reason I mentioned, layout is async and you'll see a flicker.

There are a lot of things that needs to be handled and I have spent a lot of time on it. We'd have handled it better if it was possible. It's not as simple as it looks.

I don't really like to play with the width, even less when there's no calc() in RN.

I'm talking about adding horizontal margin, not width.

We can add margins/width in the library, but it will have a flicker and there's nothing we can do about it. It's easier to add a width/margin for you because you can hardcode values depending on your app. Whether you like it or not, it will provide the best UX.

@tanoabeleyra
Copy link

I understand, thanks for taking the time to discuss it!

@fr3fou
Copy link

fr3fou commented Feb 20, 2020

image
what can i do about this in my case? do i have to wait for this PR to be merged?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants