Skip to content
This repository has been archived by the owner on Apr 18, 2024. It is now read-only.

Refactored the project #21

Merged
merged 7 commits into from
Mar 11, 2017
Merged

Refactored the project #21

merged 7 commits into from
Mar 11, 2017

Conversation

ayuhsya
Copy link
Contributor

@ayuhsya ayuhsya commented Mar 11, 2017

  • Moved all images to /resources
  • Moved all scripts to /scripts
  • Moved all data to /data
  • Moved css to /styles
  • fuse.min.js now loads from CDN instead of local storage

Copy link
Member

@athityakumar athityakumar left a comment

Choose a reason for hiding this comment

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

Hey @ayuhsya ! Thanks for opening this PR! 😄

Have a look at the changes suggested. Also, typically, the sitemap looks like this -

resources/
     images/
     js/ or scripts/
     css/ or styles/

Change the html files accordingly, to fit this the above sitemap.

add_node.rb Outdated
@@ -42,7 +42,7 @@ def default today
ARGV.clear
end

obj = JSON.parse(File.read("data.json"))
obj = JSON.parse(File.read("./data/data.json"))
Copy link
Member

Choose a reason for hiding this comment

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

Just obj = JSON.parse(File.read("data/data.json")) will do.

add_node.rb Outdated
@@ -117,10 +117,10 @@ def default today

if not testing
puts "#{obj.length} papers now."
File.delete("data.json")
File.delete("./data/data.json")
Copy link
Member

Choose a reason for hiding this comment

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

Just File.delete("data/data.json") will do.

add_node.rb Outdated
if pretty
File.open("data.json", "w") { |file| file.write(JSON.pretty_generate(obj)) }
File.open("./data/data.json", "w") { |file| file.write(JSON.pretty_generate(obj)) }
Copy link
Member

Choose a reason for hiding this comment

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

Same issue as above comment.

add_node.rb Outdated
else
File.open("data.json", "w") { |file| file.write(JSON.generate(obj)) }
File.open("./data/data.json", "w") { |file| file.write(JSON.generate(obj)) }
Copy link
Member

Choose a reason for hiding this comment

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

Same issue as above comment.

@athityakumar
Copy link
Member

@ayuhsya - No need to move the data/ folder into resources/. Just make this change, and I'll merge it, if no other changes are required.

index.html Outdated
@@ -1,10 +1,11 @@
<html>
<head>
<script src="https://code.jquery.com/jquery-2.1.4.min.js"></script>
<script src="string_score.min.js"></script>
<script src="./resources/scripts/string_score.min.js"></script>
Copy link
Member

Choose a reason for hiding this comment

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

Change all the ./resources/ in this file to resources/. And we're good to go. 👍

index.html Outdated

$.getJSON("data.json")
$.getJSON("./data/data.json")
Copy link
Member

Choose a reason for hiding this comment

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

Change this to $.getJSON("data/data.json")

@@ -1,5 +1,3 @@
importScripts('fuse.min.js');
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that the new Fuse() function in this file will work without import fuse.min.js file. We do need it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@athityakumar How can I test it?

Copy link
Member

Choose a reason for hiding this comment

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

For usual JS testing, you can check out the Console on your browser (Inspect > Console option appears). However, testing for worker js files would have to be done on server - you can push to your fork, publish that branch in your fork as a gh-pages website. Then, have a look at ayuhsya.github.io/mfqp/ and check the Console of that website.

Copy link
Member

@athityakumar athityakumar Mar 11, 2017

Choose a reason for hiding this comment

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

@ayuhsya - Or you can just restore the fuse.min.js file back, and I'll merge this. Maybe we can work on loading fuse.min.js from CDN on another PR. 😄

@amrav
Copy link
Member

amrav commented Mar 11, 2017 via email

@icyflame
Copy link
Member

@athityakumar @ayuhsya I think you guys don't know about SimpleHTTPServer 🙂

You can test everything (including JavaScript) by running:

$ python -m SimpleHTTPServer

from the mfqp/ directory and then visiting localhost:8000 on your browser.

@amrav This should fix their problem.

@ayuhsya
Copy link
Contributor Author

ayuhsya commented Mar 11, 2017

@icyflame I was using SimpleHTTPServer itself to test the HTML all this while, there's some Apache thing to create local server too, but I find it inconvenient to use.

I just can't find any usage of the file worker.js in the project that's why I asked how to test it. I'll just restore the fuse script for this PR.

@athityakumar athityakumar merged commit bdeab25 into metakgp:gh-pages Mar 11, 2017
@ayuhsya ayuhsya deleted the ref branch March 11, 2017 21:00
@amrav
Copy link
Member

amrav commented Mar 11, 2017 via email

@icyflame
Copy link
Member

@athityakumar refactor looks good. repo looks pretty now. Thanks! 😄

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants