(high priority) two push transitions corrupts tab bar (and presumably 2+)

[darenr]

After returning from a push transition, the footer tab bar operation is corrupted, switch to first tab no longer works. I verified that the tab bar urls are correct in the markup.

reproducible sequence:

• open https://kadist-ios.herokuapp.com/kmap_main
• click the 4th tab "Artists"
• click any item in the list to push transition to a new view
• click/swipe back
• click the first (left most) tab. You'll notice that the tab doesn't do anything, the return from a push transition causes Jasonette to get the tabs mixed up.

@gliechtenstein - one of my users showed me their iPhone X that did not exhibit this behavior. I wasn't able to tell if she had the latest code base but thought that this might help determine the cause (recent change in the code, or timing related)

It seems to assume that the push always happens from the first (left most) tab and restores the current view (on push view's close) to the left most tab, but that's not the tab that initiated the push. The same behaviour can be reproduced in other tabs/parts of the application too. I marked this high priority because it affects users and can't be fixed by simply pulling to refresh (because that refreshes the wrong view)

(@gliechtenstein as a temporary workaround while this is pending - switching from push to modal prevents the issue - possibly that's also useful info)

[gliechtenstein]

I was trying to look into this just now, but it's hard to test this because I've run into a couple of problems:

1. I understand you're using the modal as a workaround for now, but this means I can't test the push transition unless I replicate your entire app on my side just to change the transition to push. It would be great if you can share a markup that doesn't change so I can test it.
2. I am at step 3 and the transition is not working. The markup seems to not have a rendering action so the view just ends up being blank. Is this on production?

Anyway I'll try to fix this as soon as I can once you help me out with these issues. Thank you.

[darenr]

thanks - I've put it back to "push" and the issue reproduces for me...

not sure what you mean about not having a rendering option? Which part? Everything renders for me - which view?

[gliechtenstein]

The view doesn't render after transitioning from step 3.

As for coming back to the first tab, for me it does come back to tab 1 when i press the tab bar item after coming back from step 4.

I'm using the develop branch. Which version are you using?

[darenr]

I'm also on the develop branch. I just added

        "$load": {
          "type": "$render"
        }

but oddly for me that view renders fine without it.

Does the issue still not reproduce with the $render added?

[gliechtenstein]

I've fixed it. bba2f91

Also I've started keeping track of these small test cases just so it's easier to test every time there's a change in how tab bars work https://github.com/Jasonette/Jasontests/blob/master/tabs/index.json

Feel free to send PRs for future bugs when you encounter problems, but I can also keep updating the repo myself.

Anyway it should be fixed now, can you check?

[darenr]

awesome - just confirmed the fix works great.

[darenr]

Re-opening because the issue still reproduces with slight variation from previous steps:

• open https://kadist-ios.herokuapp.com/kmap_main
• click the 4th tab "Artists"
• click "A" item (doesn't matter which)
• click any item in the list to push transition to a new view
• click/swipe back
• click/swipe back (again)
• click the "News"(left most) tab. You'll notice that the tab doesn't do anything, the return from a push transition causes Jasonette to get the tabs mixed up.

Thanks!

[darenr]

@gliechtenstein any chance you'll have time to take a look at this please?

[gliechtenstein]

This was happening because none of the bottom tab bar items urls were matching the current url. The previous commits had fixed this to a certain extent, but this particular case was an additional layer of edge case that happened when you have multi-level hrefs.

It should be fixed now 2cdf9ef

[darenr]

thanks for taking a look, I'll merge this into my branch and give it a try.

There is no tab bar on any of the push transition views, so I'm not sure I understand the explanation. Only top-level views have the tab bar in my app, is this not an ok idea? I could change it so that all views contain the tab bar but I'm not sure that would work because the tab bar url for the currently shown view would be different each time (for each level of push transition)

What is the best practices for tab bars? I'll change to whatever you recommend and works best in the app, but my understanding was that they should only be used on the top-level view?

[gliechtenstein]

Your https://kadist-ios.herokuapp.com/kmap_artists_by_letter?letter=A has footer tab bars.

And none of the tab bar item URLs match the https://kadist-ios.herokuapp.com/kmap_artists_by_letter?letter=A

This is fine, and things like this should just work, which was the point of this entire thread and the fix.

What is the best practices for tab bars? I'll change to whatever you recommend and works best in the app, but my understanding was that they should only be used on the top-level view?

As I mentioned just above, you should be able to use tabs anywhere. That was the whole point of this fix, so no need to use it in just top-level views. Hopefully it works!

[darenr]

ok great, thanks, merging now, well, installing the multigigabyte update to xcode first. Finger's crosses xcode still works.

[darenr]

I just merged the code and unfortunately it doesn't fix the problem except for the single case where upon returning to the top-level-view I click the left most tab. If I click any other then the tabs are "confused", (mixed up, clicking one for News, opens, for example, Artists tab)

If I read you properly if I remove the tab bar from the first level on the artist's tab then it should work right, as a workaround?

I'm releasing this app soon so if there's a fix then I'll hold off, otherwise if the best solution is to remove the first level tab in the artist's tab then I can do that.

Just to be clear, steps to reproduce:

• open https://kadist-ios.herokuapp.com/kmap_main
• click the 4th tab "Artists"
• click "A" item (doesn't matter which)
• click any item in the list to push transition to a new view
• click/swipe back
• click/swipe back (again)
• click the "videos" tab
• click the "News"(left most) tab. The tab that actually get's rendered is the Artists tab (not News)

I think from looking at your code you have to maintain a stack rather than a flag, the visible tab when moving to an untabbed view should push the previous tab's url onto a stack so that as the transitions unwind the correct view is rendered.

[gliechtenstein]

Please try the latest fix c95f275 - There can be many different solutions but i've been trying to fix this while making least amount of change so as to minimize unknown factors, but historically tab bars has been really tricky.

I tried multiple different scenarios and confirmed they're working but there could be some case I overlooked, so feel free to share if there's another issue.

[darenr]

I made sure that for every push transition I replaced that tab's url with the current one (which was easier than I thought) and the tabs still get messed up after only a few clicks. By messed up I mean click "news" and get "artists" or some other tab (in this example, I've seen others playing with it for a few minutes)

I'll leave it like this then back out the tabs-on-every-view on Sunday (so it works again) in case you want to look at it before then. Here's one of many sequences that reproduces the issue:

• open https://kadist-ios.herokuapp.com/kmap_main
• click "artists" tab
• click and letter
• click any artist name
• (without clicking back) click "news" tab
• click artists tab
• click "news" tab

at this point the "news" tab shows the "artists" view

Since I've taken a lot of your time with this if you prefer not to do more on this (due to the risk of destabalizing other things) then just say and I'll only put my tabs on the top level views.

Aside: the work to replace the tab's url with the push transition's url was pretty easy with flask, but it would be possible to do this internally to Jasonette. It's definitely easier for Jasonette to be stateless and it's not that hard for users to do it but it would certainly be nice to have an option in transition like "keep_footer": "true" to auto inject the footer of the parent markup for all depth levels.

[gliechtenstein]

OK let's try this one more time. Try the latest develop. If it fails again I think it's best to just go with tab bar on level0 only for now. At least it's much better than the previous state where the only way to use tab bars was to have the current view's URL always match the tab you're in.

The stateless tabbar thing has given me too much headache and it sure is tempting to drop the policy (the majority of the buggy behaviors on Jasonette have something to do with this), but that kind of feels like selling the soul to the devil so I'll try to get this right as much as I can.

[darenr]

This is worse - now the tabs lose their push state, if for news tab I move to level 1 (from 0) then switch tabs to artists and then back I’ve lost the chevron to go back in the news tab to level 0. This happens for all tabs so the app becomes unusable because you can never get back to level 0 pretty quickly. I reverted this change in my branch and will remove the tab bar from any where other than level 0. I think that for apps using tabs there should be a top level app.json that defines the tabs so the views aren't stateless, the tabs then wouldn’t need to be defined in any of the child view levels, they would be managed at the app level. Not sure how this would work in your code but that seems like a better design. Tabs really live outside of any view. Each view belongs to a tab and all child views inherit from that tab. I don’t know iOS coding so maybe this isn’t how they work. It seems like the idea of jasonette understanding an app holistically as opposed to a set of stateless independent views might be quite challenging to change. However, I think that would be worthwhile because there are quite a few other things that are really app properties (that belong in json) but not associated with any particular view. Style classes for example would be another use for an app.json, global data is another. This though would break everyone’s apps currently. Not sure what the roadmap looks like, if you’re already planning other v2 breaking changes then might be something to consider.

…

On Jun 9, 2018, at 6:11 AM, gliechtenstein ***@***.***> wrote: OK let's try this one more time. Try the latest develop. If it fails again I think it's best to just go with tab bar on level0 only for now. At least it's much better than the previous state where the only way to use tab bars was to have the current view's URL always match the tab you're in. The stateless tabbar thing has given me too much headache and it sure is tempting to drop the policy (the majority of the buggy behaviors on Jasonette have something to do with this), but that kind of feels like selling the soul to the devil so I'll try to get this right as much as I can. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub, or mute the thread.