New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support of Open Event JSON to Giggity #56

Closed
mariobehling opened this Issue Aug 1, 2016 · 15 comments

Comments

5 participants
@mariobehling
Copy link
Member

mariobehling commented Aug 1, 2016

The repo of Giggity is here https://github.com/Wilm0r/giggity. Sample docs of the Open Event files are here https://github.com/fossasia/open-event/tree/master/sample

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 1, 2016

@mariobehling @aayusharora I would also like to work on this, if its ok with you.

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 1, 2016

@mariobehling If I understood the issue correctly, we have to modify our sample json according to the json structure of Giggity and send a Pull request to their repo right?
In this way, their Android App will allow people to browse through the details of EHealth, OTS16 and FOSSASIA16.
Am i correct?

@aayusharora

This comment has been minimized.

Copy link
Member

aayusharora commented Aug 1, 2016

@dagger I guess you are right. I understand the same by this.

@mariobehling

This comment has been minimized.

Copy link
Member

mariobehling commented Aug 1, 2016

If you look at Giggity, it supports different formats. The goal is not to change our JSON. The goal is to enable Giggity to read our format. Implement support of our format in Giggity. Create a fork and make a pull request to Giggity.

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 2, 2016

@aayusharora @mariobehling I just checked their source code.
They don't have a JSON parser yet in their app and are currently parsing only xcal/Pentabarf/frab XML schedules.

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 2, 2016

To implement support for our JSON files,we will have to modify their networking code and add json parsing support.
https://github.com/Wilm0r/giggity/blob/master/app/src/main/java/net/gaast/giggity/Schedule.java#L292
Here is the specific method where we need it to recognize JSON and then parse it accordingly.
We can start off by adding an else if block before the final else block to detect if the input is a json.
This can be done simply by checking if the input starts with [ { which is the format for our JSON files as of now.
Next up, create a method named loadJson which loops over this json and stores the keys to an Arraylist from which the contents can be extracted and assigned to the UI elements.

@siddhantbajaj

This comment has been minimized.

Copy link

siddhantbajaj commented Aug 4, 2016

@mariobehling @the-dagger @aayusharora can i take up this issue?

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 4, 2016

@siddhantbajaj Sure, go ahead

@siddhantbajaj

This comment has been minimized.

Copy link

siddhantbajaj commented Aug 5, 2016

I've solved this issue. @the-dagger can you please review the changes , After which I'll make a pull request.
https://github.com/siddhantbajaj/giggity/blob/master/app/src/main/java/net/gaast/giggity/Schedule.java

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 5, 2016

@siddhantbajaj Looks good to me.
One thing though, if there are no links associated to an event, set the link as http://fossasia.org/ rather than keeping it as null.
screenshot-area-2016-08-05-233822

@mariobehling

This comment has been minimized.

Copy link
Member

mariobehling commented Aug 5, 2016

On this screen I can see that there is no spacing to the left and right. Also if there is not link there should not be any field "links" and there should not be "null".

@mariobehling

This comment has been minimized.

Copy link
Member

mariobehling commented Aug 5, 2016

I wonder if this app needs more improvements. Well, let's go step by step.

@the-dagger

This comment has been minimized.

Copy link
Member

the-dagger commented Aug 5, 2016

@mariobehling Agreed.
@siddhantbajaj Send a separate PR which addresses these issues as well as this is common to all the json links and not just ours.

@siddhantbajaj

This comment has been minimized.

Copy link

siddhantbajaj commented Aug 5, 2016

@mariobehling @the-dagger Yes, this app needs a lot of improvement in UI. I'll fix the above issues and will send a separate PR for other issues.

@heysadboy

This comment has been minimized.

Copy link

heysadboy commented Jul 13, 2017

@mariobehling @the-dagger Added support to Giggity, find the merged pull request here Wilm0r/giggity#15 . Should we close this issue?

@the-dagger the-dagger closed this Jul 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment