Add ability to customize whether label should be rendered beneath or beside icon

[benoitdion]

Motivation

The BottomTabBar options allow for very fined grained customization but were missing a couple extension point to allow apps to override whether labels should be rendered beneath or beside the icon

Test plan

• Verify the horizontal prop is available to tabBarLabel option.
• Verify shouldRenderLabelBeneath option can be used to override the default behavior.

[benoitdion]

@satya164 @brentvatne who could help review this?

[brentvatne]

cc @satya164 - what do you think?

[satya164]

Looks good, but I'd change the option name to something simpler. Words like beneath will be prone to typos imo.

[slorber]

looks nice, but what about iconPosition or something instead? Maybe some users will want to be able to put the icon on the top/right/left/bottom

[brentvatne]

@slorber - if we use labelPosition we need to invert left and right when RTL is enabled but yeah i'd be ok with that

[benoitdion]

The current name was inspired by internal variables but I agree we can do better. How about labelOrientation?

[satya164]

I think iconPosition/labelPosition suggested by @slorber sounds the simpler. Maybe let's go with labelPosition.

[benoitdion]

went with tabDirection since it's really the direction of the content of a tab more than just the icon or label. The current implementation also only supports "left" and "top" and I'm really trying to be surgical with this change to avoid breaking any existing use cases.

[benoitdion]

any additional feedback @brentvatne @satya164?

[brentvatne]

left some comments. i don't think i'm totally on board with the tabDirection name, it's unclear to me what this means. tabBarItemOrientation might be better

[brentvatne]

this will never be a string

[benoitdion]

Err, this is supposed to be tabDirection. I'll check later but this may not be the latest commit :(. My bad

[benoitdion]

updated

[brentvatne]

this else clause will only be entered when tabDirection is falsey, and then we try to call tabDirection as a function which will error

[benoitdion]

we're checking tabDirection above

[brentvatne]

could use better type for this, unclear what it's meant to be and what the default is

[benoitdion]

modeled the type after how I'd to it in typescript. Let me know if there's a better way to do this in flow. After this goes in I may take a stab at typing more of the any's in this file.

[benoitdion]

@brentvatne updated name to tabBarItemOrientation. Let me know if there's anything else.

[satya164]

Orientation doesn't feel right to me either. It sounds like potrait or landscape orientation. IMO labelPosition: 'bottom' | 'right' is much more clear and explicit.

[benoitdion]

position is confusing as @brentvatne mentioned for when we want to have proper RTL support. It's tabBarItemOrientation so it's specific to the orientation of the content inside of the tab bar item.

[satya164]

We discussed with @brentvatne and I think tabBarPosition: 'beside-icon' | 'below-icon' sounds the best

[benoitdion]

@satya164 updated!

[brentvatne]

sorry for another change here 😅 i think that tabBarPosition is a bit confusing because we're talking about labels, so it should be labelPosition or tabItemLabelPosition, i don't have a strong preference either way between those two. the values would be the same as @satya164 mentioned above re: 'beside-icon' | 'below-icon'

[benoitdion]

@brentvatne that's a fair point. A lot of the previous naming proposals like tabDirection row/column were referring to the entire tab item since that's what this change is really about. The new naming does make it specific to having an icon and a label and how they're position relative to each other.

[satya164]

oops sorry, my mistake. I meant to say labelPosition

[slorber]

Hey. Just wonder if we shouldn't plan for supporting next having the label on right left top or bottom. I feel the 2 value names chosen would not permit to add the 2 potential other values here and may lead to a breaking change

[benoitdion]

@brentvatne @satya164 updated to labelPosition

@slorber label on the right/bottom isn't something I've seen in any app so I'm not sure we want to support it just because we can. With that said, if it becomes a use-case it's easy to add support for new values and possibly deprecate the existing ones over time. That wouldn't be a breaking change. For the time being, I like that we're constraining the names to the capabilities supported.