hippware / rn-chat

MIT License
5 stars 0 forks source link

New Navigation #974

Closed bengtan closed 7 years ago

bengtan commented 7 years ago

Follows up from: Trial migration to new React Navigation #894

Enough research and experimentation has been done that we are confident we can proceed.

We will try to split out subtasks but please keep in mind that there is more uncertainty and variability than 'regular' tickets so things may change frequently or unexpectedly. Similarly, any estimates of effort have a lower confidence interval than usual.

Some of the tickets, even though they are separate tickets, aren't implementable in isolation, and, hence, may have to be completed in clusters.

Some subtasks are for converting (groups of related) screens to use the new navigation. Some subtasks are for implementing support for specific navigational or visual elements.

@aksonov, @southerneer:

If you guys think of other things that can/should be added, please let me know, or just post a new subticket (and I can tweak the subticket's text if needed).

aksonov commented 7 years ago

I prefer not to create subtasks yet. I've just committed first version with onboarding flow with ability to login/testLogin and see home stream.

@southerneer You may improve whatever you want - add new scenes, etc. Just let me the status at the end of your working day. Tasks to do (my plan):

All such 'virtual' states should be children of 'lightbox' parent. Please check current onboarding flow. Using such practice we always could see all logic in one place and such logic will not be hidden within UI. If you don't agree, let me know.

zavreb commented 7 years ago

@aksonov it is our understanding that because everything is interdependent there is no such thing as "sub-tasks". Hence why we decided to stop our current sprints and only focus on this migration. Though these subtasks won't be completed as a whole it still helps to know the extent of the migration. Meaning one "subtask" may not be completed for weeks, and that is fine. We have allotted 2 weeks to the migration, after the 2 weeks we'll have an official check-in, it is not a "final" date. After the check-in we'll go from there.

tldr/lets not worry about the process details and lets focus on the migration itself. Let's not stress out too much since this is a heavy lift.

(cc: @bengtan, @thescurry, @southerneer, @aksonov)

southerneer commented 7 years ago
aksonov commented 7 years ago

@bengtan Maybe we need to add bot details, messages screens.

bengtan commented 7 years ago

@aksonov: Added.

aksonov commented 7 years ago

@southerneer Please check latest commit. Here are tasks:

  1. Complete navbars for Profile Detail, Bot Profile and other screen (if any) - i'm talking about dynamic right buttons:

    • Use only RNRF navbar, without 'fake' ones we have within User Profile, Bot Details (like BotNavBar), etc. With 'fake' navbars transitions don't look good and it is not correct from design point of view - so let's don't use 'hideNavBar' and then define 'navbar' as component's part anymore.
  1. Add missed screens (create bot, etc.)
aksonov commented 7 years ago

Btw, I got some error during RN0.46 upgrade: https://github.com/facebook/react-native/issues/14382 Workaround is described there (run sh script) - it works.

southerneer commented 7 years ago

OK, didn't see that error when I ran it, but dropping the solution here for reference in case I see it later... https://github.com/facebook/react-native/issues/14382#issuecomment-313163119

southerneer commented 7 years ago

@aksonov

aksonov commented 7 years ago

@southerneer

Cool! Please use onRight/rightTitle/rightImageButton from now (I've allowed to make them as functions today for beta.9) - the reason is maintenance of navbar styling - it will be the same across the app.

southerneer commented 7 years ago

@aksonov sounds good. Is there a good way to handle dynamic text styling with onRight/rightTitle/rightImageButton? For BotInfo I need the text to be 'Next', but I want to change the color dynamically.

aksonov commented 7 years ago

Will definitely add it like rightTitle today. Feel free to do it if you want

14 июля 2017 г., в 20:13, Eric Kirkham notifications@github.com написал(а):

@aksonov sounds good. Is there a good way to handle dynamic text styling with onRight/rightTitle/rightImageButton? For BotInfo I need the text to be 'Next', but I want to change the color dynamically.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

southerneer commented 7 years ago
aksonov commented 7 years ago
  1. "Name your bot" is centered.
  2. Next is not clickable - and I still see it as "Save" button, not navbar button.

Have you submitted your changes?

aksonov commented 7 years ago

I don't see any commit for https://github.com/hippware/rn-chat/tree/new-navigation, it is strange...

aksonov commented 7 years ago

@southerneer I've just completed dynamic styling as well as separate props for color (because otherwise we need to duplicate font name/size, etc) and committed different 'Next' color for Bot Create UI. It alerts '!' for active state for now, so please fix it.

Please try to commit your daily work next time so I could review-adjust it.

southerneer commented 7 years ago

@aksonov sorry about that. just committed and pushed on top of your changes.

aksonov commented 7 years ago

Could you change it to use new functionality I've committed?

17 июля 2017 г., в 18:41, Eric Kirkham notifications@github.com написал(а):

@aksonov sorry about that. just committed and pushed on top of your changes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

southerneer commented 7 years ago

@aksonov yep, I just wanted to get up quickly what I already had so you could review.

southerneer commented 7 years ago
southerneer commented 7 years ago

@aksonov I'm having trouble overriding rightButtonImage with a static rightTitle in the BotDetails component...

https://github.com/hippware/rn-chat/blob/new-navigation/src/components/BotDetails/index.js#L53

I would expect that rightTitle would display, but I still see rightButtonImage on that screen. onRight works as expected.

aksonov commented 7 years ago

Could it happen because of ‘null’ value? Have you tried ‘ ‘ (one space)?

On 19 Jul 2017, at 22:16, Eric Kirkham notifications@github.com wrote:

@aksonov https://github.com/aksonov I'm having trouble overriding rightButtonImage with a static rightTitle in the BotDetails component...

https://github.com/hippware/rn-chat/blob/new-navigation/src/components/BotDetails/index.js#L53 https://github.com/hippware/rn-chat/blob/new-navigation/src/components/BotDetails/index.js#L53 I would expect that rightTitle would display, but I still see rightButtonImage on that screen. onRight works as expected.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/hippware/rn-chat/issues/974#issuecomment-316503835, or mute the thread https://github.com/notifications/unsubscribe-auth/ABQpccJetmU7MCV1ddUE-t-lzcYnDXiyks5sPmQTgaJpZM4OQmvh.

southerneer commented 7 years ago

Ah my fault, bot.isOwn doesn't exist.

Another RNRF question. For BotCreate I need to show the back/close button, but I also want to be able to interact with the address TextInput. navTransparent shows the back button but doesn't allow interaction, hideNavBar hides the back button but does allow interaction. What should I do?

image

bengtan commented 7 years ago

An experimental and unstable version has been released. Doubtless there are many, hopefully mostly minor, issues still to be fixed. However, the major stuff seems to be done.

I would suggest that we can consider this ticket to be complete when we're ready to switch to 'new navigation', even if there are still known issues outstanding (which themselves can be tracked via separate, smaller tickets).

thescurry commented 7 years ago

Hello @bengtan (cc: @zavreb) I'm good with calling this one "complete" if Becky gives the thumbs up.

bengtan commented 7 years ago

Yay, it's closed.