Move some ngeo elements to modules #3257
Conversation
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
2 times, most recently
from
38e6b4c
to
7ca9152
Compare
|
The nego layertree work well, but the test of the layertree not. |
examples/desktopgeolocation.html
Outdated
| @@ -33,7 +33,7 @@ | |||
| ngeo-desktop-geolocation-options="::ctrl.desktopGeolocationOptions"> | |||
| </button> | |||
| <p id="desc"> | |||
| This example shows how to use the <code><a href="../apidoc/ngeo.desktopGeolocationDirective.html" title="Read our documentation">ngeo-desktop-geolocation</a></code> | |||
| This example shows how to use the <code><a href="../apidoc/ngeo..geolocation.desktop.html" title="Read our documentation">ngeo-desktop-geolocation</a></code> | |||
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
from
7ca9152
to
449a8be
Compare
|
It works :-) |
ger-benjamin
changed the title
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
2 times, most recently
from
5143329
to
700fda5
Compare
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
7 times, most recently
from
3033632
to
3e1b33b
Compare
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
3 times, most recently
from
65148e5
to
f583a31
Compare
ger-benjamin
force-pushed
the
move_some_ngeo_elements_to_modules
branch
from
f583a31
to
e164c62
Compare
|
Not all is perfect, but at least, it passes this time. Anyway, files with unclean things will move in the future |
src/directives/drawfeature.js
Outdated
| goog.require('ol.Feature'); | ||
|
|
||
| /** @suppress {extraRequire} */ | ||
| goog.require('ngeo.measure.module'); |
There was a problem hiding this comment.
That is not good. Directive code should not require a toplevel module.
Only applications and examples require top level modules. Invidivual Angular files must require the fine grained dependency they actually need.
This principle is necessary to avoid bloat. We can discuss if you think there are cases when it does not make sense.
There was a problem hiding this comment.
No you're right.
(Except eventually for some GMF directive that will be moved soon. But that's not the case here.)
|
|
||
|
|
||
| /** @type {!angular.Module} **/ | ||
| app.module = angular.module('app', [ | ||
| ngeo.module.name, | ||
| ngeo.map.module.name, | ||
| ngeo.ToolActivateMgr.module.name, | ||
| ngeo.googlestreetview.module.name, | ||
| ngeo.ToolActivateMgr.module.name |
There was a problem hiding this comment.
For information, it is now possible to have dangling comas. It helps get cleaner diffs.
There was a problem hiding this comment.
I've learned this during this PR. Thanks
|
:-) |
Create ngeo top-level modules:
I don't want to move more module at once. I'll open others PR to move the others modules