Skip to content
This repository has been archived by the owner on Aug 14, 2018. It is now read-only.

Enable offline-mode :) #13

Merged
merged 2 commits into from
Jul 4, 2015
Merged

Enable offline-mode :) #13

merged 2 commits into from
Jul 4, 2015

Conversation

Scarygami
Copy link
Member

No description provided.

this._apiUser = undefined;
this._localUser = {};
this._isAuthorized = false;
this._setUser({});
Copy link
Contributor

Choose a reason for hiding this comment

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

function does not exists

Copy link
Member Author

Choose a reason for hiding this comment

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

Polymer creates those private property setters for you, to be used for read-only properties. See https://www.polymer-project.org/1.0/docs/devguide/properties.html#read-only

Copy link
Contributor

Choose a reason for hiding this comment

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

K forgot about those...
but the I think there is no need for that call, the user property is computed and should get notified of the change of _googleUser to undefined which gets handled correctly by the computation function.
I made the comment because Chrome raised a js error on that line.
Let me double check if everything works as expected without the set or ... we need to open an issue
Edit: forgot about all of this... seems like my bower update didn't download the correct version of polymer....... now everything works properly

Copy link
Member Author

Choose a reason for hiding this comment

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

Btw, I tested this again just now, and setting _googleUser to undefined doesn't execute _computeUser again.

I think generally computed properties/bindings only get evaluated if all parameters are != undefined (which caused me several problems already...)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there is and issue regarding undefined... but obviously I can't find the issue...
Instead I've found this TL;DR; we should try out if setting the objects to null instead of undefined triggers the computation correctly and the conditions still works correctly

Copy link
Member Author

Choose a reason for hiding this comment

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

I think I tried that originally, but still had the issue of user and with
it the menus not updating correctly. Would need some more testing and
possibly a bug to issue with the Polymer team.

On Mon, Jul 6, 2015, 09:52 Mauro Solcia notifications@github.com wrote:

In app/elements/gde-signin/gde-signin.html
#13 (comment)
:

  •        name: profile.getName(),
    
  •        image: profile.getImageUrl(),
    
  •        email: profile.getEmail(),
    
  •        token: authResponse.access_token || authResponse.id_token
    
  •      };
    
  •    },
    
  •    /**
    
  •     \* User signed out, remove all user information
    
  •     */
    
  •    _signedOut: function () {
    
  •      this._googleUser = undefined;
    
  •      this._apiUser = undefined;
    
  •      this._localUser = {};
    
  •      this._isAuthorized = false;
    
  •      this._setUser({});
    

I think there is and issue regarding undefined... but obviously I can't
find the issue...
Instead I've found this Polymer/polymer#1813
TL;DR; we should try out if setting the objects to null instead of
undefined triggers the computation correctly and the conditions still works
correctly


Reply to this email directly or view it on GitHub
https://github.com/GoogleDeveloperExperts/experts-app-web/pull/13/files#r33912042
.

SmokyBob added a commit that referenced this pull request Jul 4, 2015
@SmokyBob SmokyBob merged commit 4a06de9 into GoogleDeveloperExperts:master Jul 4, 2015
@Scarygami Scarygami deleted the offline branch July 5, 2015 19:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants