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.

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

Bug: Clicking on pre-selected tab shows empty fragment instead of base fragment #29

Closed
p-fischer opened this issue Nov 21, 2016 · 24 comments
Assignees

Comments

@p-fischer
Copy link

p-fischer commented Nov 21, 2016

After the long awaited release, I could finally get rid of frag-nav as a locale module and include it as a dependency. Thank you for that! :)

Unfortunately, I found an issue occurring in both versions 1.1.0 and 1.2.0:

Previously I could click on a tab, which already was selected, and go back to the first fragment of the tab's stack. No matter if there were one or multiple fragments on the stack.

Now, I just see an empty fragment.

I assume this is a bug or must the library be used differently now?

@bulatgaleev
Copy link

bulatgaleev commented Nov 21, 2016

Same.
Also it happens when i pop() while there's only 1st fragment of 1st tab in stack.

@ncapdevi ncapdevi self-assigned this Nov 21, 2016
@ncapdevi
Copy link
Owner

@localhostEU This is actually intentional. Sometimes different apps do different things when you get to the bottom of the stack, and I wanted to leave the flexibility up to the user of the library. If you want to back out of the activity at the bottom of the stack, you'll want to do something like this:


    @Override
    public void onBackPressed() {
        if (mNavController.canPop()) {
            mNavController.pop();
        } else {
            super.onBackPressed();
        }
    }

@ncapdevi
Copy link
Owner

@p-fischer So sorry for the delay on things. I'm back to actively managing the project. Could you show me the code that you're using on a re-selection of the tab in order to clear the stack?

@p-fischer
Copy link
Author

p-fischer commented Nov 21, 2016

Glad, you're back on active development since I think this lib perfectly captures my desired behavior for bottom bar navigation.

This is my code. On re-selection, I'm basically just calling clearStack() - it should be equal to what you do in your sample app.

        bottomBar.setItemsFromMenu( R.menu.menu_bottombar, new OnMenuTabClickListener()
		{
			@Override
			public void onMenuTabSelected( @IdRes int menuItemId )
			{
				awaitSecondBackPress = true;

				View lastSelectedTab = findViewById( lastSelectedTabId );
				View nowSelectedTab = findViewById( menuItemId );

				if ( lastSelectedTab != null )
				{
					lastSelectedTab.setSelected( false );
				}

				if ( nowSelectedTab != null )
				{
					nowSelectedTab.setSelected( true );
				}

				lastSelectedTabId = menuItemId;

				switch ( menuItemId )
				{
					case R.id.bottombar_1:
						navController.switchTab( INDEX_1 );
						break;
					case R.id.bottombar_2:
						navController.switchTab( INDEX_2 );
						break;
					case R.id.bottombar_3
						navController.switchTab( INDEX_3 );
						break;
				}
			}

			@Override
			public void onMenuTabReSelected( @IdRes int menuItemId )
			{
				lastSelectedTabId = menuItemId;
				navController.clearStack();
			}
		} );

@ncapdevi
Copy link
Owner

Hmm, seems to be working for me. Looks like you're using an older version of bottombar library. Could you try updating to the latest version

compile 'com.roughike:bottom-bar:2.0.2'

Then your code would look like this for the re-selection

        bottomBar.setOnTabReselectListener(new OnTabReselectListener() {
            @Override
            public void onTabReSelected(@IdRes int tabId){
		lastSelectedTabId = menuItemId;
                navController.clearStack();
            }
        });

@p-fischer
Copy link
Author

Updating to bottom-bar v2 could be the solution.

However, since my app is close to its next release I won't be able to test that immediately.

I guess your sample app using bottom-bar v1 could run into the same issue. You might want to update it too.

@ncapdevi
Copy link
Owner

@p-fischer That's a good point, I wasn't running into it with the sample app, so there must be something else going on. I need to push out a bug fix, so I'll need to release 1.2.1 sometime today anyways, I'll look into it more and see if I can find exactly what's going on. Could you show me how you're adding your fragments?

@ncapdevi
Copy link
Owner

It's strange to me that this could be happening at all. The first lines of the clearStack() method are

    public void clearStack() {
        //Grab Current stack
        Stack<Fragment> fragmentStack = mFragmentStacks.get(mSelectedTabIndex);

        // Only need to start popping and reattach if the stack is greater than 1
        if (fragmentStack.size() > 1) {}

Meaning that it will only attempt to clear the stack if it has more than one fragment on it. And you're saying if you are on a tab, and then reselect it, it's clearing it to empty?

@p-fischer
Copy link
Author

List<Fragment> fragments = new ArrayList<>( 3 );
fragments.add( FirstFragment.newInstance( false ) );
fragments.add( SecondFragment.newInstance() );
fragments.add( ThirdFragment.newInstance( false ) )

navController = new FragNavController( savedInstanceState, getSupportFragmentManager(),
                                          R.id.container, fragments, INDEX_1 );

where INDEX_1 = 0
called from onCreate()

@ncapdevi
Copy link
Owner

Does INDEX_1 =0; or does INDEX_1 = FragNavController.TAB1; ?

@bulatgaleev
Copy link

What's v2? Theres nothing about it in description/changelogs.

@ncapdevi
Copy link
Owner

@p-fischer
Copy link
Author

@ncapdevi the second option. Does it make a difference?

@localhostEU v2 refers to this library used in the sample app: https://github.com/roughike/BottomBar

@bulatgaleev
Copy link

@ncapdevi @p-fischer Ah, that was bottom bar context :) Nvm then!

@ncapdevi
Copy link
Owner

@p-fischer It shouldn't, but the annotation processor might have been doing something weird if it was just set to 0 (since it's supposed to only allow the predefined TabIndex). That looks entirely right. I'll see if I can replicate it. Just to clarify again. If you go from Tab 1 -> Tab 2 and press Tab 2 again, it clears the stack to an empty state? and also, if you add fragments on to Tab 1, and re-select that tab, it's also clearing the stack to an empty state?

@ncapdevi
Copy link
Owner

@p-fischer One other question, are you adding fragments to the container in any way other than using the navController object?

@p-fischer
Copy link
Author

@ncapdevi Yes I saw the "empty fragment" in both cases - with a current stack supposed to be of size 1 and with size 2 before re-selection.
However, I did not debug into the issue and look into the current stack after the re-selection.

@ncapdevi
Copy link
Owner

I'll see if I can reproduce it, but any other information would be helpful.

@p-fischer
Copy link
Author

p-fischer commented Nov 21, 2016

One other question, are you adding fragments to the container in any way other than using the navController object?

No, fragments are pushed to the stack through this method of MainActivity exclusively:

@Override
public void pushFragment( Fragment fragment )
{
	if ( navController != null )
	{
		navController.push( fragment );
	}
}

As soon as I look into it again, I'll share any information.

@p-fischer
Copy link
Author

@ncapdevi I'm using another version of the bottom-bar lib in my project than your sample app does:
compile 'com.roughike:bottom-bar:1.3.4'

@ncapdevi
Copy link
Owner

Able to reproduce it. Will let you know when I can get and push out a fix. Thanks for your help

@ncapdevi ncapdevi added the bug label Nov 21, 2016
@ncapdevi
Copy link
Owner

@p-fischer fixed it. https://github.com/ncapdevi/FragNav/releases
@localhostEU This bug was also affecting you in the back press, but is fixed now.

compile 'com.ncapdevi:frag-nav:1.2.1'

@ncapdevi
Copy link
Owner

Just realized the issue isn't entirely fixed. It was will clear the stack, but not all the way to the bottom. I'll push out a fix again today

@ncapdevi ncapdevi reopened this Nov 21, 2016
@ncapdevi
Copy link
Owner

Actually, the problem shouldn't affect you with the way the constructor that you are using. That being said. I've pushed out a fix.

compile 'com.ncapdevi:frag-nav:1.2.2'

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

No branches or pull requests

3 participants