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

Working UI #1542

Merged
merged 1 commit into from
May 4, 2017
Merged

Working UI #1542

merged 1 commit into from
May 4, 2017

Conversation

maxwo
Copy link

@maxwo maxwo commented May 3, 2017

In case you're interested, here is the corrected/working version of the UI compared to the current state on master.
Things I did:

  • Add babel-loader to make the UglifyJS working with ES2015 source;
  • Continue the work begun in the provider service.

@ldez ldez added the area/webui label May 3, 2017
@ldez ldez self-requested a review May 3, 2017 16:38
@ldez ldez added this to the 1.3 milestone May 3, 2017
Copy link
Contributor

@ldez ldez left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 👍

angular
.module(traefikCoreProvider, ['ngResource'])
.factory('Providers', Providers);
angular.module(traefikCoreProvider, ['ngResource']).factory('Providers', Providers);
Copy link
Contributor

Choose a reason for hiding this comment

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

please restore the initial code format.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, my fault. I may have auto-format too quickly.

@@ -1,11 +1,26 @@
const conf = require('./gulp.conf');
const proxy = require('http-proxy-middleware');
Copy link
Contributor

Choose a reason for hiding this comment

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

could you explains/remove this dependency ?

Copy link
Author

Choose a reason for hiding this comment

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

This dependency allows you to run in dev mode with a real Traefik backend running on localhost:8080.
For instance, when you run :
docker run -p 8080:8080 -p 80:80 -v /var/run/docker.sock:/var/run/docker.sock traefik --debug --web --docker --docker.watch
and then
npm run serve
On http://localhost:3000/ , the server is now able to connect to the running traefik /api and /health endpoints.
I think it could be useful for testing with a real server, and since it is in dev dependencies, it is not bundled in the final release. Let me know if you want me to remove this.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please remove this part for now, and create an other PR for this. thanks.

Copy link
Contributor

Choose a reason for hiding this comment

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

One PR, one subject 😉

Copy link
Contributor

@ldez ldez May 3, 2017

Choose a reason for hiding this comment

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

But I like this feature.


return rawProviders;
get: function() {
return $q((resolve, reject) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use Promise instead of $q

Copy link
Author

@maxwo maxwo May 3, 2017

Choose a reason for hiding this comment

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

$q is the preferred way to handle promises with AngularJS (1.X) , since it wraps the promise mechanism into a $digest loop, which allows immediate view refresh. For instance, notice how it takes 2 additional seconds to display data on startup, before it effectively does the $interval call that effectively refreshes data.

@maxwo
Copy link
Author

maxwo commented May 3, 2017

I just pushed the style correction, plus changed a test to a cleaner one that keeps displaying providers even if there are no backend registered to that provider.

I'd be please to contribute by migrating to an Angular, React or VueJS front-end if I have time and you are up to, with unit tests (sorry, haven't add time to add this in this PR since there wasn't any originally, and I just wanted to make the UI work again...). Let me know.

@ldez
Copy link
Contributor

ldez commented May 3, 2017

If you want migrate to Angular or VueJS, we will be very happy 😃 . (And create a new PR for this: one PR, one subject. 😉 )

@ldez ldez added the kind/enhancement a new or improved feature. label May 3, 2017
@maxwo maxwo mentioned this pull request May 3, 2017
chore(Build) : Add Babel for build.
chore(Babel) : Add babel configuration.
style(Code) : Enhance code style.
@ldez ldez merged commit 6ad273b into traefik:master May 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/webui kind/enhancement a new or improved feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants