search

CodeLanka / ez-net-app

EZ Net is an application developed for rural internet users which are not tech savy and shows interest in the internet and the resources it has.
MIT License
15 stars 29 forks source link

Fetch data from Firebase Database #31

Closed TheLukaszNs closed 6 years ago

TheLukaszNs commented 6 years ago

What this PR can help with?

Well, this PR fixes #29 and adds new Api class which can be developed along with adding new Firebase functionality.

But wait!

I think we need to change/rebuild Firebase database structure (and then some of Api.js), because it doesn't contain important information (like thumbnail) or is structured incorrectly (for example Sites/Newspapers returns:

{
 "දිවයින":{"url":"www.divaina.com"},
 "ලංකාදීප":{"url":"www.lankadeepa.lk"}
}

instead of (like it's in dummy_api.js):

[
 {"id": 1, "title":"දිවයින", "thumbnail": "NEWSPAPERS_DIVAINA", "url":"www.divaina.com},
 .
 .
]

(I know it's hard to make such a data structure in Firebase 😞) so I had to correct it manually in the code which makes it a little bit less reader firendly). There's one more bug: look at line 44 in Api.js

I'm waiting for review, for now: Happy Xmas! 🎄

Yeah, it probably needs a bigger review and next patch to fix this Database structure

TheLukaszNs commented 6 years ago

And one more important note: I had tested it only on Android

agentmilindu commented 6 years ago

Wonderful job!

agentmilindu commented 6 years ago

@nushydude Hope you too can go through this.

TheLukaszNs commented 6 years ago

I think that database is now structured better, but before I push next commit to my PR: wouldn't it be nice if users could see categories icons, too? We should add thumbnails fields to categories, because these questions marks isn't looking so good. Take a look at this gist.

Also, in my opinion, it'd be much better for code when the URLs had contain https:// or http:// prefixes.

agentmilindu commented 6 years ago

@TheLukaszNs yes, indeed, let's add category thumbnails too. I will add the HTTP protocols to the URLs also.

TheLukaszNs commented 6 years ago

@agentmilindu I had updated Api.js with new Firebase fields, added temporary CATEGORY_COOKING image (CC0 license) and deleted this unnecessary check in App.js.

agentmilindu commented 6 years ago

@TheLukaszNs Awesome! BTW, the PR says me there is a conflict in dummy_api.js, thus I can't merge this. Can you resolve the conflict, please? Maybe because it is I changed the base to develop. Can you pull from develop and resolve the conflicts?