FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix-next(tabview): visually pre-load tab items on android by MartoYankov · Pull Request #5495 · NativeScript/NativeScript · GitHub

fix-next(tabview): visually pre-load tab items on android - #5495

Merged
MartoYankov merged 3 commits into
masterfrom
tab-view-fragments
Mar 8, 2018
Merged

MartoYankov merged 3 commits into
masterfrom
tab-view-fragments

Conversation

Copy link
Copy Markdown
Contributor

No description provided.

MartoYankov self-assigned this Mar 6, 2018
ghost added the in progress label Mar 6, 2018
ns-bot added the cla: yes label Mar 6, 2018
return this._viewPager.getOffscreenPageLimit();
}
[androidOffscreenTabLimitProperty.setNative](value: number) {
console.log("----> androidOffscreenTabLimitProperty: " + value);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Just noticed when looking at new PRs, probably didn't mean to include this.

MartoYankov Mar 6, 2018 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

👍 thanks ^^

transaction.remove(this._currentEntry.fragment);
transaction.commitAllowingStateLoss();

this._currentEntry.fragment = null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Removing currentEntry.fragment while animation is not completed will trigger problems with application restore.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Actually, this fixes an issue with application restore. In the scenario - Tabs - Frames - Pages - all of the Frames fragments are restored before the Tabs fragments are recreated and they try to load their pages before they have been selected. The solution we came up with is to remove the Frame fragment on unload of the frame. A new problem came up - on suspend the Frame is unloaded after saveInstanceState, so the fragments are not really removed according to Android and it tries to recreate them on resume. That is why we set the currentEntry's fragment to null, so that on fragment create we can set it's contents to null, making it a non UI fragment. When the Frame is then loaded it will create a new fragment for the page.

Basically, we want the Android to re-create the Tab fragment, which loads its Frame which creates a new fragment for the Page. I'm open to other ideas.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I couldn't reproduce the problem that nullifying the fragment was supposed to fix, so I removed it 👍

const entry = this.entry;

if (!entry) {
// On app suspend the Frames that are unloaded destroy their fragments after

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

You need to match restored fragment to Page or TabItem otherwise you will have a blank pages in some cases when application is restored.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is connected to the currentEntry fragment assigned to null. It handles this case.

// TODO: addCheck if the fragment is visible so we don't load pages
// that are not in the selectedIndex of the Tab!!!!!!
if (!page.isLoaded) {
if (frame.isLoaded && !page.isLoaded) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

if frame.isLoades is false will only make page load later and it will break page transitions (pages will be empty)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you share a test case, so that I can check the problem?

const index = tabView.items.indexOf(this);
// Don't load items until their fragments are instantiated.
if (index === tabView.selectedIndex && (<TabViewItemDefinition>this).canBeLoaded) {
if ((<TabViewItemDefinition>this).canBeLoaded) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This will load several views (before and after the selectedIndex). The whole point was an improvement so that we fire loaded only for the current selectedIndex.
Firing loaded may trigger another events like navigation, etc...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Unfortunately, for Android we have to load as much views as the ViewPagerAdapter creates. Otherwise the swipe gesture of the TabView shows blank page before the navigation is completed. CanBeLoaded is assigned by the ViewPagerAdapter instantiate item method. We have also enabled androidOffscreenLimit to be equal to 0, which enables the scenario you talk about. The idea is to have it 0 when the TabView is on the bottom and there is no swipe to navigate.

ADjenkov self-requested a review March 8, 2018 09:15

Copy link
Copy Markdown
Contributor Author

test branch_cuteness#tab-view-fragments

lock Bot commented Aug 26, 2019

Copy link
Copy Markdown

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

lock Bot locked and limited conversation to collaborators Aug 26, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants


Back | FazBrowse Home | New Git URL