moved config to http-vue-loader #4374

OmgImAlexis · 2018-06-15T09:00:16Z

sharkykh · 2018-06-15T12:32:03Z

IMO, we have to do something about the fact it loads the config from the API twice.

One from core.js and the other from the store.

Edit: Oh, we can't :(

sharkykh

I'm sorry but I think I'm against moving to http-vue-loader.
It seems to only complicate things right now.
And it adds unnecessary overhead due to our current configuration.
We should do the transitions in steps.
I think we should convert all pages to the mako-vue files we've been doing until now first.

.

sharkykh · 2018-07-02T06:24:56Z

medusa/server/web/config/handler.py

@@ -2,16 +2,7 @@

 """Base handler for Config pages."""

-from __future__ import unicode_literals


__future__ imports should usually stay. This one in particular.
Basically, they're the first steps of making Python 2 compatible with Python 3.
Sorry for not being more explicit...

sharkykh · 2018-07-02T06:28:03Z

themes-default/slim/static/js/templates/config.vue

+    computed: store.mapState(['config']),
+    mounted() {
+        const { $store } = this;
+        $store.dispatch('getConfig');


You didn't respond to my review about that.
I don't understand why we still need to get the config if we enabled it globally with the mixin?

I have to look into it but for some reason it's not getting the config set to anything but null like it is when we first load the store.

I removed that code and it still works fine for me? 🤔

Who knows then. I remove it.

OmgImAlexis · 2018-07-02T06:42:55Z

After trying to convert more templates locally I found that the jQuery tabs refuse to load if a Vue HTML element is found inside of it and if I move the component back to being loaded locally via the x-templates it works fine.

sharkykh · 2018-07-02T07:08:38Z

@OmgImAlexis
Have you tried calling the tabs initializations ($(element).tabs() IIRC) on the component's mounted()?

OmgImAlexis · 2018-07-02T07:39:23Z

Yes, I also tried commenting it out and running it after everything has loaded via the console and nothing happens.

Stale.

sharkykh

Final suggestions...

sharkykh · 2018-07-02T17:26:27Z

themes-default/slim/static/js/templates/config.vue

+
+module.exports = {
+    name: 'config',
+    computed: store.mapState(['config']),


I don't know if it affects performance, but this mapState is not needed.

I still want to keep it there as I'd like to disable the mixin for .vue files.

sharkykh · 2018-07-02T17:34:23Z

themes-default/slim/static/js/templates/config.vue

@@ -0,0 +1,64 @@
+<template>
+    <div id="config-content">


How do you feel about:

<div v-if="!config.pythonVersion" class="vue-loading" id="config-content"> <h3>Loading...</h3> </div> <div v-else id="config-content">

It gives it a cleaner look in the meantime, but a little more work when it's no longer needed.
I added class="vue-loading" so if we keep it the same on all .vue files it would be easy to remove them.

I was thinking we could just add a loading component before the <router-view>?

<loading v-if="loading"></loading> <router-view v-else></router-view>

How would you know when the values were loaded?

For example on this you'd set loading: true on data and then set a function to watch config when it updates set loading: false.

sharkykh · 2018-07-02T17:36:06Z

themes-default/slim/static/js/lib/http-vue-loader.js

@@ -0,0 +1,470 @@
+(function umd(root,factory){


I still prefer a minimised version of this file, unless you believe it would be easier to debug issues?

Yeah, I think for now it'll be easier.

sharkykh · 2018-07-03T00:19:06Z

themes/dark/templates/config.mako

@@ -1,122 +1,5 @@
 <%inherit file="/layouts/main.mako"/>


~~These files~~ config.mako didn't get deleted on dark and light themes, need to manually remove

OmgImAlexis · 2018-07-03T23:26:52Z

@sharkykh could you please do a final review.

sharkykh

@OmgImAlexis

sharkykh · 2018-07-04T04:53:47Z

themes-default/slim/static/js/router.js

+            component: notFoundComponent
+        }, {
+            path: '*',
+            redirect: '/not-found'


On develop, navigate to http://localhost:8081
Page is loaded but URL is now http://localhost:8081/not-found
It's caused by this redirection.

Not getting this issue on dev with and without webroot so I'm not sure what's causing it on your end.

Do you maybe still have the service worker loaded? Try disabling it by going to the Application menu in Chrome's dev-tools and hitting Clear site data.

I don't use a webroot. ServiceWorker is loaded but I don't think it has anything to do with it.

Try navigating from say, /home to /schedule by clicking the Schedule nav link. You should get this issue.
Don't know why it only seems to happen on /schedule route.

That 100% has something todo with it. Remove the service worker.

Oh, I just figured it out.

Medusa/themes/light/templates/schedule.mako

Line 15 in 9eaf648

router,

We need to remove this line.

Oh.. no we just need to add the route for it. I'm moving the headers and titles over to the router.js. Just don't add a component property and it works fine.

For example.

routes: Object.assign(routes || [], [{ path: '/config', name: 'config', meta: { title: 'Help & Info', header: 'Medusa Configuration' }, component: configComponent }, { path: '/home', name: 'home', meta: { title: 'Home', header: 'Show list' } }, { path: '/history', name: 'history', meta: { title: 'History', header: 'History' } }, { path: '/not-found', name: 'not-found', meta: { title: '404', header: '404 - page not found' }, component: notFoundComponent }, { path: '*', redirect: '/not-found' }])

We then use this in the pages that aren't .vue since they don't use the global Vue.

header() { return this.$route.meta.header; },

themes-default/slim/static/js/templates/config.vue

+    store, // eslint-disable-line no-undef
+    computed: {
+        config() {
+            return this.store.state.config;


OmgImAlexis requested a review from sharkykh June 15, 2018 09:00

OmgImAlexis added Enhancement Concluded Needs review labels Jun 15, 2018

OmgImAlexis added this to the 0.2.6 milestone Jun 15, 2018

sharkykh reviewed Jun 15, 2018

View reviewed changes

This comment has been minimized.

Sign in to view

OmgImAlexis modified the milestones: 0.2.6, backlog Jun 25, 2018

OmgImAlexis added 2 commits July 1, 2018 14:59

moved config to http-vue-loader

bfc5487

switch config component to vue SFC

ff118dc

OmgImAlexis force-pushed the move-config-to-http-vue-loader branch from e5f8096 to ff118dc Compare July 1, 2018 05:31

finished conversion

f535c3d

pymedusa deleted a comment from todo bot Jul 1, 2018

OmgImAlexis added this to the 0.2.7 milestone Jul 2, 2018

OmgImAlexis added 4 commits July 2, 2018 15:26

fix config for review

dec4597

remove .min from filename

a9bbc9d

remove unused code

d2c0cc7

remove .min from script path

3a45f4d

sharkykh previously requested changes Jul 2, 2018

View reviewed changes

add missing __future__ import

460cba6

OmgImAlexis mentioned this pull request Jul 2, 2018

cleanup app-link for tests #4526

Merged

remove unneeded dispatch and sync themes

90e3046

OmgImAlexis added Concluded and removed In progress labels Jul 2, 2018

sharkykh mentioned this pull request Jul 2, 2018

Feature/add vue tests #4527

Merged

sharkykh previously approved these changes Jul 2, 2018

View reviewed changes

sharkykh reviewed Jul 3, 2018

View reviewed changes

OmgImAlexis added 2 commits July 3, 2018 11:35

remove old config.mako

b76c8bd

move computed config to this.store

6963ada

OmgImAlexis dismissed sharkykh’s stale review via 6963ada July 3, 2018 02:06

lint

ebe47e5

sharkykh approved these changes Jul 3, 2018

View reviewed changes

OmgImAlexis merged commit b88db29 into develop Jul 4, 2018

OmgImAlexis deleted the move-config-to-http-vue-loader branch July 4, 2018 02:27

sharkykh reviewed Jul 4, 2018

View reviewed changes

OmgImAlexis mentioned this pull request Jul 4, 2018

fix store typo #4555

Merged

sharkykh mentioned this pull request Jul 5, 2018

Fix schedule page navigating to /not-found #4602

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

moved config to http-vue-loader #4374

moved config to http-vue-loader #4374

OmgImAlexis commented Jun 15, 2018

sharkykh commented Jun 15, 2018 •

edited

Loading

sharkykh left a comment

This comment has been minimized.

sharkykh Jul 2, 2018

sharkykh Jul 2, 2018

OmgImAlexis Jul 2, 2018

sharkykh Jul 2, 2018

OmgImAlexis Jul 2, 2018 •

edited

Loading

OmgImAlexis commented Jul 2, 2018

sharkykh commented Jul 2, 2018 •

edited

Loading

OmgImAlexis commented Jul 2, 2018

sharkykh left a comment

sharkykh Jul 2, 2018

OmgImAlexis Jul 3, 2018

sharkykh Jul 2, 2018

OmgImAlexis Jul 3, 2018 •

edited

Loading

sharkykh Jul 3, 2018

OmgImAlexis Jul 3, 2018 •

edited

Loading

sharkykh Jul 2, 2018

OmgImAlexis Jul 3, 2018

sharkykh Jul 3, 2018 •

edited

Loading

OmgImAlexis commented Jul 3, 2018

sharkykh left a comment

sharkykh Jul 4, 2018

OmgImAlexis Jul 4, 2018 •

edited

Loading

sharkykh Jul 4, 2018 •

edited

Loading

OmgImAlexis Jul 4, 2018

sharkykh Jul 4, 2018

OmgImAlexis Jul 4, 2018

OmgImAlexis Jul 4, 2018 •

edited

Loading

This comment was marked as resolved.

		@@ -2,16 +2,7 @@

		"""Base handler for Config pages."""

		from __future__ import unicode_literals

moved config to http-vue-loader #4374

moved config to http-vue-loader #4374

Conversation

OmgImAlexis commented Jun 15, 2018

sharkykh commented Jun 15, 2018 • edited Loading

sharkykh left a comment

Choose a reason for hiding this comment

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmgImAlexis Jul 2, 2018 • edited Loading

Choose a reason for hiding this comment

OmgImAlexis commented Jul 2, 2018

sharkykh commented Jul 2, 2018 • edited Loading

OmgImAlexis commented Jul 2, 2018

sharkykh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmgImAlexis Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmgImAlexis Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sharkykh Jul 3, 2018 • edited Loading

Choose a reason for hiding this comment

OmgImAlexis commented Jul 3, 2018

sharkykh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmgImAlexis Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

sharkykh Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OmgImAlexis Jul 4, 2018 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

sharkykh commented Jun 15, 2018 •

edited

Loading

OmgImAlexis Jul 2, 2018 •

edited

Loading

sharkykh commented Jul 2, 2018 •

edited

Loading

OmgImAlexis Jul 3, 2018 •

edited

Loading

OmgImAlexis Jul 3, 2018 •

edited

Loading

sharkykh Jul 3, 2018 •

edited

Loading

OmgImAlexis Jul 4, 2018 •

edited

Loading

sharkykh Jul 4, 2018 •

edited

Loading

OmgImAlexis Jul 4, 2018 •

edited

Loading