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

[MIG][9.0] template: Upgrade JS API #208

Merged
merged 1 commit into from Nov 4, 2016

Conversation

lasley
Copy link
Contributor

@lasley lasley commented Jul 28, 2016

Milestone (Odoo version)

  • v9

Module(s)

  • template

Fixes / new features

  • Upgrade JS to new API
  • Upgrade onload method to use core bus web_client_ready
  • Add an exported method

REF OCA/web#386 (comment)

cc @antespi @pedrobaeza

@pedrobaeza
Copy link
Member

Thanks for this update

👍

@antespi
Copy link
Contributor

antespi commented Jul 29, 2016

👍 Thanks @lasley

@pedrobaeza
Copy link
Member

@moylop260, there's a strange error in Travis. Do you have an idea of it?

@moylop260
Copy link
Contributor

I saw the same error from stable branch build
https://travis-ci.org/OCA/maintainer-tools/builds/147765216

It is a traceback from flake8

I'll use travis2docker to debug and fix it..

@moylop260
Copy link
Contributor

FYI I can reproduce it with t2d using the travis original image (with other environment I can't reproduce it)

The version with problem flake8 3.0.2
screen shot 2016-07-29 at 9 27 07 pm

I install a old version 2.6.2 and work fine!
screen shot 2016-07-29 at 9 29 05 pm

I will create a new PR with a freeze of version


core.bus.on('web_client_ready', null, function () {
// Script that will be loaded when document is ready
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This approach doesn't work for website. Instead, the proper way to determine that the document is ready seems to be via ready() in web_editor.base, which returns a Promise object. This means that for website, the example here should look more like:

var base = require('web_editor.base');
base.ready().done(function() {
    // Script that will be loaded when document is ready
});

Copy link
Member

Choose a reason for hiding this comment

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

This is not used in website, but on web client (as stated). Maybe we should include both in 2 files: one for web, and another for website.

Copy link
Contributor Author

@lasley lasley Aug 11, 2016

Choose a reason for hiding this comment

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

I see no reason to not include as many examples as possible. @pedrobaeza you're thinking a separate file would be good instead of just putting both here and commenting the difference?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I'm thinking about 2 files, naming then web_module_name.js and website_module_name.js

@lasley
Copy link
Contributor Author

lasley commented Aug 18, 2016

Split website sample into separate file and added a web_ namespace to the web ones & rebased onto master to fix conflicts.

I did not add a website tour file, because I am not quite sure of the intricacies between the back and front end tours (if any). IMO it would be bad for me to add a sample I am not 100% positive of 😄

@gurneyalex
Copy link
Member

👍

* Upgrade JS to new API
* Upgrade onload method to use core bus web_client_ready
* Add an exported method
* Create website module and namespace web modules
@lasley
Copy link
Contributor Author

lasley commented Nov 3, 2016

Squashed. This is good for merge?

@pedrobaeza pedrobaeza merged commit abb5ba4 into OCA:master Nov 4, 2016
@lasley lasley deleted the bugfix/9.0/js9-api branch November 4, 2016 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants