Skip to content

Vue admin application#410

Merged
stefanoborini merged 48 commits into
masterfrom
vue-admin
May 8, 2017
Merged

Vue admin application#410
stefanoborini merged 48 commits into
masterfrom
vue-admin

Conversation

@stefanoborini

Copy link
Copy Markdown
Contributor

Replace the full administrative interface with a Vue based implementation.

// install router
Vue.use(VueRouter);

const router = new VueRouter({

@martinRenou martinRenou Apr 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The linter will complain about the const, only available in ES6


const app = new Vue({
router
}).$mount('#app');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If I understand correctly, it's equivalent to set the "el" attribute like this:

const app = new Vue({
    el: "#app",
    router: router
})

});

const app = new Vue({
router

@martinRenou martinRenou Apr 28, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Only available in ES6, should be router: router

mounted: function() {
resources.Stats.retrieve()
.done((function(rep) {
this.$data.realm = rep.realm;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this.realm also works, idem for all data attributes

this.$emit('closed');
},
createNewUser: function() {
var user_name = $.trim(this.name);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't understand why JQuery implements the trim function... It's equivalent to the Javascript method this.name.trim(), am I right ? So we should use the built in method instead of depending on a library for that.

name: items[id].name
});
});
this.$data.users = users;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this.users

close: function () {
this.$emit('closed');
},
fieldClassName: function (field) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

should be in the template directly, it will reduce your code:

:class="{ 'has-error': formstate.name && (formstate.name.$touched || formstate.name.$submitted) && formstate.name.$invalid }"

It's one long line vs 11 lines

});
this.containers = containers;
}).bind(this))
.fail(function () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line can be removed

});
this.users = users;
}).bind(this))
.fail(function () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can be removed

`,
props: ['accToRemove'],
methods: {
close: function () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No need for a close method, you can put this in the template directly to save some lines of code

<div class="modal-body">Do you want to remove accounting {{ accToRemove.id }}?</div>

<div class="modal-footer text-right">
<button type="button" class="btn btn-default" @click="close">Cancel</button>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@click="$emit('closed')"

@codecov-io

codecov-io commented May 5, 2017

Copy link
Copy Markdown

Codecov Report

Merging #410 into master will increase coverage by 0.03%.
The diff coverage is 95.91%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #410      +/-   ##
==========================================
+ Coverage    95.3%   95.33%   +0.03%     
==========================================
  Files          92       88       -4     
  Lines        4090     4077      -13     
  Branches      257      259       +2     
==========================================
- Hits         3898     3887      -11     
+ Misses        141      138       -3     
- Partials       51       52       +1
Impacted Files Coverage Δ
remoteappmanager/admin_application.py 100% <ø> (ø) ⬆️
remoteappmanager/handlers/api.py 100% <ø> (ø) ⬆️
...anager/handlers/admin/tests/test_admin_handlers.py 100% <100%> (ø) ⬆️
...oteappmanager/handlers/admin/admin_home_handler.py 100% <100%> (ø) ⬆️
...teappmanager/webapi/admin/tests/test_accounting.py 100% <100%> (ø) ⬆️
remoteappmanager/webapi/admin/accounting.py 94.91% <93.54%> (+7.73%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ac19521...1ba8397. Read the comment docs.

@@ -21,4 +21,4 @@ def get(self):
num_images=len(app.db.list_applications()),
num_running_containers=len(containers)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

all these entries in the context must go away.

]
});

Vue.component("modal",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

modal-component

@stefanoborini stefanoborini changed the title Vue admin [WIP] Vue admin application May 8, 2017
@stefanoborini stefanoborini merged commit cc6fd77 into master May 8, 2017
@stefanoborini stefanoborini deleted the vue-admin branch May 8, 2017 15:28
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.

3 participants