Closed devKiratu closed 1 year ago
@brianwachira please review
The expected response from the server is:
{
"data": [
{
"id": 1,
"name": "Droidcon Ke",
"email": "droidconke@gmail.com",
"description": "droidcon Ke",
"facebook": "droidconke",
"twitter": "droidcon.co.ke",
"instagram": "droidcon.co.ke",
"logo": "http://droidcon-erp.herokuapp.com/upload/logo/apppng.png",
"slug": "droidcon-ke-645",
"status": "active",
"created_at": "2021-09-10 16:26:32",
"upcoming_events_count": 1,
"total_events_count": 1
}
],
"meta": {
"paginator": {
"count": 1,
"per_page": "15",
"current_page": 1,
"next_page": null,
"has_more_pages": false,
"next_page_url": null,
"previous_page_url": null
}
}
}
Would be nice if this pr included the types.
The expected response from the server is:
{ "data": [ { "id": 1, "name": "Droidcon Ke", "email": "droidconke@gmail.com", "description": "droidcon Ke", "facebook": "droidconke", "twitter": "droidcon.co.ke", "instagram": "droidcon.co.ke", "logo": "http://droidcon-erp.herokuapp.com/upload/logo/apppng.png", "slug": "droidcon-ke-645", "status": "active", "created_at": "2021-09-10 16:26:32", "upcoming_events_count": 1, "total_events_count": 1 } ], "meta": { "paginator": { "count": 1, "per_page": "15", "current_page": 1, "next_page": null, "has_more_pages": false, "next_page_url": null, "previous_page_url": null } } }
Would be nice if this pr included the types.
Hey @makunomark , thanks for the sample response.
The types are a good enhancement, I'll add them. My thinking is that this component is using the id
and logo
properties only from the data received, the interface/type would only contain those. What are your thoughts on this?
Generally great change. We are going to be querying what to display from an API, I have attached a json response sample in a separate comment. Do you think its a great idea to add types, and redux to await for when we will have set up the api?
Yes, these are good ideas. I'll implement
The expected response from the server is:
{ "data": [ { "id": 1, "name": "Droidcon Ke", "email": "droidconke@gmail.com", "description": "droidcon Ke", "facebook": "droidconke", "twitter": "droidcon.co.ke", "instagram": "droidcon.co.ke", "logo": "http://droidcon-erp.herokuapp.com/upload/logo/apppng.png", "slug": "droidcon-ke-645", "status": "active", "created_at": "2021-09-10 16:26:32", "upcoming_events_count": 1, "total_events_count": 1 } ], "meta": { "paginator": { "count": 1, "per_page": "15", "current_page": 1, "next_page": null, "has_more_pages": false, "next_page_url": null, "previous_page_url": null } } }
Would be nice if this pr included the types.
Hey @makunomark , thanks for the sample response.
The types are a good enhancement, I'll add them. My thinking is that this component is using the
id
andlogo
properties only from the data received, the interface/type would only contain those. What are your thoughts on this?
Definately, we want to have and use only what we need to use
Generally great change. We are going to be querying what to display from an API, I have attached a json response sample in a separate comment. Do you think its a great idea to add types, and redux to await for when we will have set up the api?
Hey @makunomark ,
I have added the types in accordance with the expected data.
As for redux, I've opted to use local state management for this component using the useState
hook and load the data when the component mounts in the useEffect
hook. My reasoning was as follows:
Redux is best suited for global state that changes often. The data for this component is very local and is as static as it can get, and therefore elevating it to the redux store would, in my opinion, pollute the store without getting much value from this.
That said, I am open to hearing your thoughts/suggestions on this.
@devKiratu Looks good.
Thanks @makunomark . I'm participating in the hacktoberfest challenge. Does this PR qualify? I'd appreciate if it is added to my tally
Description
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
This PR extracts the
Organized by
code into a reusable component thus avoiding repetition of code. The component is shared by the following files:Minor bug fixes/improvements/enhancements were also made on the
HomeScreenNotLoggedIn.tsx
fileFixes #46
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Screenshots
HomeScreenNotLoggedIn with modal not visible
HomeScreenNotLoggedIn with modal visible
HomeScreen - (Signed in)![Screenshot 2022-10-18 at 11-07-45 WhatsApp Image 2022-10-18 at 11 05 55 jpeg (JPEG Image 720 × 1520 pixels) — Scaled (46%)](https://user-images.githubusercontent.com/59362946/196374096-94072deb-bcf8-48e9-8a82-e64c01ccc060.png)
AboutScreen![Screenshot 2022-10-18 at 11-08-31 WhatsApp Image 2022-10-18 at 11 05 56 jpeg (JPEG Image 720 × 1520 pixels) — Scaled (46%)](https://user-images.githubusercontent.com/59362946/196374084-830b096a-bc4f-468a-b253-276c453c6537.png)
Checklist: