Alpha website candidate #462
Conversation
|
Good work 😄 First simple suggestion: fix the column width to avoid column resize on search. Thank you for pushing this idea forward 😄 |
Definitely! Few suggestions:
I'll review the code later on :) |
src/index.html
Outdated
| <meta name="viewport" content="width=device-width, initial-scale=1.0"> | ||
| <link rel="stylesheet" type="text/css" href="styles.css"/> | ||
| <link rel="shortcut icon" href="favicon.ico" type="image/x-icon"/> | ||
| <script src="https://unpkg.com/vue"></script> |
There was a problem hiding this comment.
Let's use the minified version of Vue https://unpkg.com/vue@2.4.2/dist/vue.min.js
There was a problem hiding this comment.
Also the JS should be in the footer ;)
src/scripts.js
Outdated
| @@ -0,0 +1,72 @@ | |||
| loadJSON(function(response) { | |||
There was a problem hiding this comment.
We shouldn't need to wait to load the JSON before the page kicks in, this will slow down the initial rendering. I'd say we opt for promises too.
There was a problem hiding this comment.
YOu can use the created() function on the Vue instance itself to load the data, so everything thing else can initialise while you wait.
Like this 😄 https://codepen.io/mikestreety/pen/jLYGbW
src/scripts.js
Outdated
| xobj.send(null); | ||
| } | ||
|
|
||
| function filterRows() { |
There was a problem hiding this comment.
I have a feeling this can be done with Vue itself. Leave it with me, I'll pull down the branch and have a play later.
There was a problem hiding this comment.
I've not looked at the code too much, but could the filtering be done like this?
src/styles.css
Outdated
| @@ -0,0 +1,48 @@ | |||
| body { | |||
| border-collapse: collapse; | |||
| font-family: "Trebuchet MS", Arial, Helvetica, sans-serif; | |||
There was a problem hiding this comment.
Can we use a nicer font-stack, maybe something from Google Fonts?
|
Sorry if everyone got loads of notifications - I cocked up the review ;) Sorted now! |
|
Thanks for the great feedback everyone! I've made the following improvements:
I will update S3 as well. |
src/index.html
Outdated
| <div id="app"> | ||
| <div class=center-text> |
src/index.html
Outdated
| </body> | ||
| <footer> |
There was a problem hiding this comment.
We don't need these tags.
|
Side note - I've never used Vue.js before, but I'm liking the functionality so far! |
|
Merging this in to start second iteration. |
|
:) |
I've uploaded my alpha site candidate to an S3 bucket, viewable here. It utilizes Vue over JQuery and Bootstrap, minimizing libraries and crazy amounts of minified file fun. I'm more than happy to listen to suggestions on this plan, but I am going to keep pressing forward in this direction. There is still plenty of work to be done here, such as loading the JSON file locally to speed up the table rendering.