clseibold / ImportantZites

List of Important Zites for ZeroNet.
http://127.0.0.1:43110/15Pf9VVuDT8NSWj1qUBh4V89yPmrmzRw6a/
MIT License
0 stars 0 forks source link

Adds automated category-loading, and some other changes.. #1

Closed AnthyG closed 7 years ago

AnthyG commented 7 years ago

WARNING Dear @krixano, please create a backup of content.json and data/data.json! Also, you have to change your the name of the property category_name in categories of your data/data.json-File to name, as it is self-explaining that the name is meant to be the categories name.

I furthermore recommend doing a backup of the complete Zite, just in case anything goes wrong!

Now to the fun part: The major change coming with this pull-request is the automated loading of the categories, placed in the data/data.json-File. This resulted in the removal of many lines and thus, much cleaner code.

I also don't want to guarantee the functionality of this code, as I can't verify that it actually works.

AnthyG commented 7 years ago

Should I also merge the search-feature-Branch of mine into my master? Or shall I just do a new pull-request for that ❔

ghost commented 7 years ago

Maybe do a different pull request for that - I'm not sure. I've never collaborated on any projects before. I would say a different one so that we keep pull requests for different features separate. What do you think is best?

AnthyG commented 7 years ago

Yes, I agree with that 👍

AnthyG commented 7 years ago

I would then recommend, to create a backup of your Zites directory, as I deleted some important files like the Zites content.json.

I will create the other pull-request, when you merge this one.

ghost commented 7 years ago

I'm currently seeing a bug where the zites don't load until I sign the content.json file - but when I refresh, I have to resign it to load the zites again. There also seems to be a problem with the tabClick function. I am looking into these things right now.

ghost commented 7 years ago

I'm getting an error on the "this.cmd()" function call in the loadZites() function saying that "this" is undefined.

AnthyG commented 7 years ago

I'll fix that very quick (the "this"-problem).

AnthyG commented 7 years ago

I don't know, what the bug with the Zites not loading is.. But I also have that problem with the tabClick-Function, resulting in entries not getting hidden, when a tab is clicked 😞

ghost commented 7 years ago

How would I commit changes on this pull request? Is that allowed? I haven't used pull requests all that much before.

AnthyG commented 7 years ago

I honestly don't know.. 🤔 You'd probably either have to edit it after merging, or fork my fork, edit it, do a pull-request, I accept it, and it appears here.. 😆

I'm not an git-expert either, so I can't really tell...

AnthyG commented 7 years ago

Oh, it seems, I deleted an if-statement that I didn't replace..: https://github.com/krixano/ImportantZites/blob/master/index.html#L258

AnthyG commented 7 years ago

Any more issues/ errors/ mistakes?

AnthyG commented 7 years ago

The entries don't seem to get hidden, when another tab is selected 💢 Oh well, that's because I am not in ZeroNet..

ghost commented 7 years ago

Now I'm getting an unexpected token } error when clicking on a tab, but it doesn't tell me the line it's on

AnthyG commented 7 years ago

Do you get it, when you just load the Zite? Or does it happen, when you do a specific action :?

ghost commented 7 years ago

When clicking on a tab

AnthyG commented 7 years ago

I'll just add ; everywhere.. Not expecting it to be fixed though.

AnthyG commented 7 years ago

Well that took some time, I don't really know why.. Also fixed a little thing, where I forgot to escape the '

ghost commented 7 years ago

Two more issues:

  1. Clicking on a tab doesn't set the previous one as not active if the previous tab is the 'all' tab
  2. After a refresh, clicking on a tab works correctly, however, clicking on another tab doesn't (but does set the previous one as not active - so the 'All' tab is never set as not active).
AnthyG commented 7 years ago

I'll look into this.. today.. but like in 14+h :laughing: (or earlier, if I can)

ghost commented 7 years ago

I found out how I can also commit changes on this pull request. Basically, I was able to commit and push directly to your fork because you forked my repository. You can find out more about it here: https://help.github.com/articles/committing-changes-to-a-pull-request-branch-created-from-a-fork/

ghost commented 7 years ago

So now, one last bug to solve - the "All" tab isn't set as not active when clicking on another tab.

AnthyG commented 7 years ago

Today I won't be much at home, but as far as I checked the code from my phone, everything should be fine, so I don't know what keeps the "All"-Tab from not hiding >:/

AnthyG commented 7 years ago

When doing this (I have to do it, as I'm not able to clone the Zite), tab-changing works just fine 🤔

getCategoriesList(cb) {
    if (typeof cb === "function")
        cb([{
            "name": "info"
        }])

    // page.cmd("dbQuery", ["SELECT * FROM categories"], (cats) => {
    //     if (typeof cb === "function")
    //         cb(cats);
    // })
}

Have you changed the data/data.json-Files category-property category_name to name, and reloaded and rebuilt your Database? ... Has to be, as you can obviously see the category-tabs 😅

ghost commented 7 years ago

Another thing, redoing a database query for each tab click to get the categories seems inefficient (and will lag older computers - like mine for example), so we should cache the categories in a variable. Then, in the getCategories function, check if the variable is not null and call the callback with it as parameter, and if it is null, do the database query and store the categories in the variable so that next time the function is called, the variable will not be null (and will be directly given to the callback without having to do another query).

AnthyG commented 7 years ago

Maybe select it when the document is ready? Either document.onready = function(){...} , or a self-executing function (function(){...})

ghost commented 7 years ago

No, that didn't work. Two things I noticed:

  1. Putting toggleClass(document.getElementById('allTab'), "is-active") in the onOpenWebsocket function right before the categories are loaded does toggle the class on the allTab element.
  2. However, putting previousActiveTab = document.getElementById('allTab'); in the onOpenWebsocket function right before the categories are loaded doesn't fix anything.

I think this could mean that it has something to do with the variable itself, and not the document.getElementById('allTab') part.

But, the variable works for all of the other tabs and for the all tab only after another tab has been selected at least once.

AnthyG commented 7 years ago

Then maybe triggering a click-event on the all tab fixes it? ..That's bullsh*t..

So maybe leaving both of the variables blank at the start, and then calling the tabChange-function with "all"? .. Or doing the thing I said before, that I striked through too?

Lol.

ghost commented 7 years ago

That works. There is just one small thing, though it isn't that big of a deal, which is that it takes like a second for the tab to highlight/select.

AnthyG commented 7 years ago

@krixano one can't select a tab twice anyways.. so it's technically fixed :laughing:

ghost commented 7 years ago

Okay, so I think I am ready to merge this now.

AnthyG commented 7 years ago

Yay :tada::smile: