Skip to content
This repository was archived by the owner on Nov 10, 2017. It is now read-only.

Update kept to work with the new daybed.js#26

Open
almet wants to merge 3 commits into
n1k0:daybed_storagefrom
almet:daybed_storage
Open

Update kept to work with the new daybed.js#26
almet wants to merge 3 commits into
n1k0:daybed_storagefrom
almet:daybed_storage

Conversation

@almet
Copy link
Copy Markdown

@almet almet commented Sep 1, 2014

This uses the new daybed.js API and works with kept to share the dashboards, using a hash in the url.

Comment thread src/js/kept.js
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Isn't this operation idempotent ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It is, yes, but I don't understand what's the matter here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do you compare the current value then ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This says: "If the current session is different from the one of the hash, then set the hash to the new value".

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I mean : what's the problem with overwriting with the same value ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah, I prefer avoid doing so as I don't know what that triggers for the browser (e.g. if there is no need to, why should we update it?)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok, no problem dude ! Just FYI onhashchange is called only if the value changes :)

Comment thread src/js/store/daybed.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't get this : each user will have its own model ?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Each token, yes.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you explicit pros and cons of this pattern please ? Maybe in Daybed documentation page about permissions ?

( only one I can foresee is about managing permissions, but TBH I find it weird )

@almet
Copy link
Copy Markdown
Author

almet commented Sep 2, 2014

@n1k0 do you know if the failures are due to the daybed.js changes or aren't related?

Comment thread src/js/store/daybed.js Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We might have to choose a way to indent then() consistently...
I think I prefer this way:

return Daybed.startSession(self._options.host, {
    token: this._options.token
  })
  .then(function(session) {
    // ...
  });

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