Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[NavigationExperimental] NavigationHeader should let the user override container styles #7204

Closed
tlvenn opened this issue Apr 25, 2016 · 4 comments
Closed

[NavigationExperimental] NavigationHeader should let the user override container styles #7204

tlvenn opened this issue Apr 25, 2016 · 4 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@tlvenn
Copy link
Contributor

tlvenn commented Apr 25, 2016 •
edited

Right now, the NavigationHeader let the user overrides the appbar style but not the following ones which are hardcoded and cant be changed:

title: {
    bottom: 0,
    left: APPBAR_HEIGHT,
    marginTop: STATUSBAR_HEIGHT,
    position: 'absolute',
    right: APPBAR_HEIGHT,
    top: 0,
  },

  left: {
    bottom: 0,
    left: 0,
    marginTop: STATUSBAR_HEIGHT,
    position: 'absolute',
    top: 0,
  },

  right: {
    bottom: 0,
    marginTop: STATUSBAR_HEIGHT,
    position: 'absolute',
    right: 0,
    top: 0,
  }

I am honestly not sure why left / title / center are not leveraging flexbox and are positioned in absolute, probably to take care of the title position and size which might follow strictly IOS guideline however it does have some drawbacks:

  • We cant really maximise the width of the title, it's fixed instead of being fluid and we cant put the title closer to the back button either.
  • Given that the title area is fixed, it can easily overlay over the right region if the latter has more than 2 buttons for example, should all those containers use flexbox, it would not be possible.

I would like to propose to add a subviewStyles prop that would contain style properties for title, left and right so people can reset and change the default styles for those containers.

I also wonder if using flexbox would not be more flexible out of the box for people.

@ericvicenti any though ?

Thanks
@ericvicenti
Copy link
Contributor

We cannot use flexbox because of the fancy animations that people expect in the header. We are missing some logic to properly measure the title and re-position things accordingly. At this point, if you want a flexbox based header, I think it makes the most sense to fork the existing header and publish an independent version.

The prop you want to add makes sense, to allow overriding of styles like marginTop, which are shared between the header items. Feel free to send in a PR with that additional prop, and tag me so I don't miss it. Thanks!

@tlvenn tlvenn mentioned this issue Apr 25, 2016
Closed
@tlvenn
Copy link
Contributor Author

tlvenn commented Apr 25, 2016

Done, thanks for the quick feedback and the explanation on why flexbox is currently not used.

ghost pushed a commit that referenced this issue Apr 25, 2016
@tlvenn
Add subviewStyle prop to NavigationHeader
1dc82a9 
Summary:
Implements: #7204

Pinging ericvicenti for review.
Closes #7214

Differential Revision: D3219306

Pulled By: ericvicenti

fb-gh-sync-id: becd1dada2557b7fb2c345bac2098094fa6d2144
fbshipit-source-id: becd1dada2557b7fb2c345bac2098094fa6d2144
This was referenced Apr 26, 2016
Closed
Closed
ptmt pushed a commit to ptmt/react-native that referenced this issue May 9, 2016
@tlvenn @ptmt
Add subviewStyle prop to NavigationHeader
a72fa46 
Summary:
Implements: facebook#7204

Pinging ericvicenti for review.
Closes facebook#7214

Differential Revision: D3219306

Pulled By: ericvicenti

fb-gh-sync-id: becd1dada2557b7fb2c345bac2098094fa6d2144
fbshipit-source-id: becd1dada2557b7fb2c345bac2098094fa6d2144
zebulgar pushed a commit to nightingale/react-native that referenced this issue Jun 18, 2016
@tlvenn @zebulgar
Add subviewStyle prop to NavigationHeader
3afdcd1 
Summary:
Implements: facebook#7204

Pinging ericvicenti for review.
Closes facebook#7214

Differential Revision: D3219306

Pulled By: ericvicenti

fb-gh-sync-id: becd1dada2557b7fb2c345bac2098094fa6d2144
fbshipit-source-id: becd1dada2557b7fb2c345bac2098094fa6d2144
@ryankask
Copy link
Contributor

There is an issue on Android with the NavigationHeader styles. The absolutely positioned left and right subview containers only specify the position of 3 of the component's sides.

It seems like if you render any absolutely positioned content using renderRightComponent or renderLeftComponent, nothing shows because the parent has no width. On iOS the content forces a width.

2016-07-29 at 15 58

I expect to see the button rendered on Android but cut off because there is no support for overflow: 'visible'.

samerce pushed a commit to iodine/react-native that referenced this issue Aug 23, 2016
@tlvenn @samerce
Add subviewStyle prop to NavigationHeader
35dff37 
Summary:
Implements: facebook#7204

Pinging ericvicenti for review.
Closes facebook#7214

Differential Revision: D3219306

Pulled By: ericvicenti

fb-gh-sync-id: becd1dada2557b7fb2c345bac2098094fa6d2144
fbshipit-source-id: becd1dada2557b7fb2c345bac2098094fa6d2144
@lacker
Copy link
Contributor

lacker commented Oct 28, 2016

The original issue seems to be resolved here so this issue has gotten a bit unwieldy and is discussing multiple things. I'm going to close this issue but if there's still a bug in the navigation library then I encourage people to open a new issue with repro code for it.

@lacker lacker closed this as completed Oct 28, 2016
@facebook facebook locked as resolved and limited conversation to collaborators Jul 19, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@ryankask @lacker @ericvicenti @tlvenn @react-native-bot