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

Display text immediately on mount #4

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@slorber
Contributor

slorber commented Feb 5, 2018

Former implementation does delay the display of the text.

This means that if you use the ticker in a phrase, the phrase will first render with ticker hidden, and then the ticker will be displayed.

For simple string childs it's not necessary to wait, we can simply display the text directly as a "preview" of the ticker.

For simple text childs: display text immediately
Former implementation does delay the display of the text.

This means that if you use the ticker in a phrase, the phrase will first render with ticker hidden, and then the ticker will be displayed.

For simple string childs it's not necessary to wait, we can simply display the text directly as a "preview" of the ticker.
@slorber

This comment has been minimized.

Contributor

slorber commented Feb 5, 2018

Basically I use the ticker like that:

const PriceHeaderContent = ({textPrefix,price}) => (
  <View flexDirection="row" alignItems="center">
    <BaseText s greyLL>{textPrefix}</BaseText>
    <PriceTicker s greyLL price={price}/>
    <BaseText s greyLL></BaseText>
  </View>
);

And on mount the ticker text is currently hidden, while the rest of the text is visible which feels a bit weird, so displaying the text immediately on mount is better UX for me.

@slorber

This comment has been minimized.

Contributor

slorber commented Feb 5, 2018

Note that it would probably be easy to support that text preview in case of a childs array, by concatenating all the chars of the ticks childrens to get the text preview (I mean, extracting the text from JSX tree)

I can try to support that too if you guarantee that the ticker children is always an array of ticks (or at least elements that take a single char a children)

@slorber slorber changed the title from For simple text childs: display text immediately to Display text immediately on mount Feb 5, 2018

@browniefed

This comment has been minimized.

Owner

browniefed commented Feb 5, 2018

Cool, I'll take a look at this later. My only concern would be a jump in the text, but technically we're measuring the height so there shouldn't be any issue.

@slorber

This comment has been minimized.

Contributor

slorber commented Feb 5, 2018

I've tested in my app and currently see no jump (on Android device/Expo at least)

For weird reasons however I had to put "backgroundColor: transparent" on the style prop otherwise I get the digits overflowing the ticker container, can't understand why (maybe it's a local problem, tell me if you see it)

Do you want me add support for initial text for tick childs? I mean using something like const text = React.Children.toArray(childs).reduce(concatChars) does it make sense?

@slorber

This comment has been minimized.

Contributor

slorber commented Feb 21, 2018

Hi, tell me if I can do anything additional to get this merged ;)

@browniefed

This comment has been minimized.

Owner

browniefed commented Feb 21, 2018

Ah sorry, I haven't had a chance to review this. I moved on from native app for now and am working on web hence (https://github.com/browniefed/react-flip-ticker). I'll get a chance in a week or 2.

@slorber

This comment has been minimized.

Contributor

slorber commented Apr 10, 2018

Hi @browniefed , did you have time to review the PR or do you want me to change anything?

@slorber

This comment has been minimized.

Contributor

slorber commented Jun 11, 2018

ping :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment