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

Fixed to show spinner on data load #1491

Merged
merged 2 commits into from Oct 10, 2018

Conversation

Projects
None yet
2 participants
@stephanwlee
Copy link
Collaborator

stephanwlee commented Oct 5, 2018

Also fixed the issue where, when showing the spinner, the iron-collapse
does not always open. Note that this collapse bug did exist in version
1,10 too,

Fixed to show spinner on data load
Also fixed the issue where, when showing the spinner, the iron-collapse
does not always open. Note that this collapse bug did exist in version
1,10 too,
@nfelt

nfelt approved these changes Oct 10, 2018

@@ -120,7 +119,8 @@ export const DataLoaderBehavior = {

if (!this.isAttached) return;
this._loadDataAsync = this.async(() => {
this.dataLoading = true;
// Using a setter does not notify the impl observers.

This comment has been minimized.

Copy link
@nfelt

nfelt Oct 10, 2018

Collaborator

Hmm, this is basically the same issue as the loadKey issue, right? So it seems like maybe the behavior observer/property connection is buggy or broken somehow.

@@ -140,7 +140,7 @@ export const DataLoaderBehavior = {
});

return Promise.all(promises).then(this._canceller.cancellable(result => {
this.dataLoading = false;
this.set('dataLoading', false);

This comment has been minimized.

Copy link
@nfelt

nfelt Oct 10, 2018

Collaborator

Can you add the same comment here?

@@ -58,7 +58,6 @@ export const DataLoaderBehavior = {

dataLoading: {
type: Boolean,
readOnly: true,

This comment has been minimized.

Copy link
@nfelt

nfelt Oct 10, 2018

Collaborator

Does this have to be non-readOnly in order to use set()? It's still logically readonly though, right?

This comment has been minimized.

Copy link
@stephanwlee

stephanwlee Oct 10, 2018

Author Collaborator

Ah, you were correct! I always forget that you ned to use a special setter when setting a value to a readOnly property. I changed other codes to use this._setDataLoading and kept this readOnly. Thanks!

@stephanwlee stephanwlee merged commit 44c830f into tensorflow:master Oct 10, 2018

2 checks passed

cla/google All necessary CLAs are signed
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@stephanwlee stephanwlee deleted the stephanwlee:spin branch Oct 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.