Library structure

[marcelpinto]

Hi,

First nice job doing this library, but there are few things I would change.

1. Make it more flexible, I do not see the reason why to create the bar dynamically in java. This is a ui library, thus should be provided as xml widget. Why? well, it makes it dynamic, flexible and visually to have it on the xml, the dev will be able to decide if wants to have some other view on top, for example coordinator layout? what happen here with the scroll behavior? etc...
2. I would keep it simple! I don't see the reason why the BottomBar should be handling fragments. It should only be responsible of creating the tabs and the proper animations for each item as well as notifying on event. That's the problem of some UI libraries, they try to do too much.
3. Related to the first point. Create and style_attribute so the dev can set a menu from xml.
4. Add ability to set the full color of the BottomBar by tab, so a tab should define, icon, text, color

If I have so time this weekend I maybe fork it and make some pull request.

Br,
Marcel

[HugoGresse]

I agree, as it's a UI library, it should be added via the layout. Maybe you can refer to how works NagvationView and use menu xml to populate the Bottombar for example?

[roughike]

@skimarxall
Good points.

1. The creation dynamically by Java code was to simplify the use. I found it a bit awkward to use it directly by xml, as you would have to use a FrameLayout with two children (your main content and BottomBar with android:layout_gravity="bottom" set) to get started. I'll look into using it by XML though, as it's been requested multiple times.
2. I think it's kind of a matter of taste issue, but don't worry: I won't implement completely irrelevant things. I just think that handling Fragments automatically is convenient.

3 & 4. Will do.

[roughike]

@HugoGresse
A NavigationView is just a FrameLayout and as far as I can tell, doesn't do much regarding it's position on screen. It's usually wrapped in a DrawerLayout which handles putting the NavigationView to the right place.

[AlexLionne]

yes right ! java should be used only to override xml attributes

[marcelpinto]

@roughike I think the position of the bar should be a job of the dev. I know it's nice to have everything out of the box, but normally this leads to workarounds to fix some special cases.

Take a look at this gist https://gist.github.com/NikolaDespotoski/1d6fef4949eb9be05a46

He is using the support TabLayout wrapped in a LinearLayout as a BottomNavigation. I think you BottomBar should replace that. So a normal view will look like

<android.support.design.widget.CoordinatorLayout
    xmlns:android="http://schemas.android.com/apk/res/android"
    xmlns:app="http://schemas.android.com/apk/res-auto"
    android:layout_width="match_parent"
    android:layout_height="match_parent">

    <android.support.v4.widget.NestedScrollView
        android:layout_width="match_parent"
        android:layout_height="match_parent">

        <TextView
            android:layout_width="match_parent"
            android:layout_height="match_parent"
            android:text="@string/long_text"/>
    </android.support.v4.widget.NestedScrollView>

    <com.roughike.bottombar.BottomBar
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_gravity="bottom"
        app:tabMenu="@menu/bottom_menu"
        app:layout_behavior="@string/bottom_navigation_behavior"
        app:tabLayoutId="@+id/tabs"/>
</android.support.design.widget.CoordinatorLayout>

Maybe you could even avoid the width and height and force it on the creation of your view. But I am not sure about that.

[roughike]

@skimarxall
I forgot to mention the other problem with xml inflation. In your example, the NestedScrollView wouldn't know anything about the BottomBar (and vice versa), so the BottomBar would overlap content, because the NestedScrollView would be partially under it.

[marcelpinto]

@roughike that's how it has to be. This is the behaviour defined by the specs. When you scroll the bottom navigation should go away. thus the scroll content should get the full size. Is the dev that needs to let enough padding or spacing at the end in order to avoid content overlapping.

Or you use a RelativeLayout and the nested scroll view defines layout_above="@+id/bottom_bar"

That's is what I actually meant with being flexible, is the dev job to decide this kind of behavior.

[roughike]

@skimarxall
Alright, fair points.

I'm making it usable by XML and I'll provide instructions how to use it in the Readme.

[roughike]

Here's some solutions for your issues:

1. No XML (see below why), but by using bottomBar.attach(findViewById(R.id.myView), savedInstanceState) will let you control what View the BottomBar should contain. Basically with this you can put it anywhere in the View hierarchy.
2. Not really an issue, and lots of people are already using it (including myself) so no reason for removing this feature.
3. & 4. These are both now supported.

Here's what I found about the XML issue:

• A bit awkward to use it directly by xml, as you would have to use a RelativeLayout with two children, your main content with android:layout_above="@+id/bottomBar" and BottomBar with android:layout_alignParentBottom="true" set to get started.
• Even after that it won't behave properly on tablets without extra configuration from the user (an extra layout in layout-sw600dp folder). It should look like this but the BottomBar has no way to not overlap content.

Using the BottomBar should be as simple as possible and same code should work by default for both tablets and phones solution.

For these reasons, I'm closing this issue.

I really appreciate your feedback though! Feel free to create new issues if something new comes to your mind.

[marcelpinto]

@roughike how would you then use it with coordinator layout if its not on the xml?

At the end is a library decision if you want to give it as xml or java. I think since it's a view it makes more sense to use it as xml even if then it requires some extra configuration.

Glad that you implement the other two points though.