search

GroceriStar / showcase

Simple page with Grocery List cards
https://boring-benz-76e55f.netlify.com/
GNU General Public License v3.0
1 stars 5 forks source link

Handle TODO comments #133

Open Avuidrauxs opened 5 years ago

Avuidrauxs commented 5 years ago

This issue was opened to handle tasks related to all the TODO comments in the project

Avuidrauxs commented 5 years ago
screen shot 2018-12-28 at 3 03 06 am

@atherdon Hasn't this been achieved already , from the TODO, i'm reading

atherdon commented 5 years ago

@Avuidrauxs can you add a codeline to each of this todos, so i can quickly review it and undestand a context? like this: https://github.com/GroceriStar/showcase/blob/master/src/App.js#L4-L8

for this case - yeah - just delete a todo comment and next import of homeview. Note: please fork showcase and apply your changes via pull request

Avuidrauxs commented 5 years ago

@atherdon , okay understood. I have updated the issue description with links now as requested.

Avuidrauxs commented 5 years ago

@atherdon , Can you clarify what you want me do with reworm over here ? https://github.com/GroceriStar/showcase/blob/master/src/App.js#L12,

atherdon commented 5 years ago

@Avuidrauxs let's skip a reworm for now. or spend some time and generate an explanation for me how you'll do it and how it'll benefit to us. because i didn't read a lot about it.

atherdon commented 5 years ago

find some paper notes, related to showcase project- will add them as tasks for future at issues. tell me if you pick some of this todos or i should pick it for you. just not sure what the status is

atherdon commented 5 years ago

I'm also jumping into this project for a few days, so be ready to my avalanche of small commits :(

Avuidrauxs commented 5 years ago

@atherdon , I'm on them. But I'll be wary of your commits đŸ˜„

Avuidrauxs commented 5 years ago

@atherdon , What kinda links method from react-router you want me to use in https://github.com/Avuidrauxs/showcase/blob/master/src/InsideLayout.js#L17,

Do you want me to refactor the function to use <Links> or rather use history.push() from History to redirect to onClick of the button.

Avuidrauxs commented 5 years ago

@atherdon , Also yeah I will see if reworm is still relevant for what we want to achieve.

atherdon commented 5 years ago

@Avuidrauxs we should connect somehow the getLink function with our router component. I mean - it's bad that we hardcoded this path '/grocery/:id' . let's assume this situation: you change links at router, for example to '/grocerylist/:id' and after this chages - getLink method is actually a bug, and only if you remember about it - you'll fix it.

atherdon commented 5 years ago

this todo can be done pretty quick too: https://github.com/GroceriStar/showcase/blob/master/src/components/Cell/InsideLayout.js#L22-L30

Avuidrauxs commented 5 years ago

@atherdon What states should I check before rendering the InsideLayout, https://github.com/GroceriStar/showcase/blob/master/src/components/Cell/InsideLayout.js#L22-L30

Avuidrauxs commented 5 years ago

@atherdon Also on the context of reworm with React's new Context API, we probably could stick with that since it will be less complex for other interns to pick up on the topic of state management. I would like to iron out more details and debate this with you but what do you think of React's Context API ?

atherdon commented 5 years ago

i like an idea of Context API. @Avuidrauxs can you give me details about what do you want to debate of :)

Btw, in the last weeks we have some updates with our project. We still need a lot of things todo, but this task with this codelines should be updated. And I want to see some code updates from you as well ;) if you want - we can create a small separated task, where we'll have only one of TODOs - that you'll be able to solve quickly

Avuidrauxs commented 5 years ago

@atherdon , Yaah but I think first of all I want to know the ideal way you want new interns to be able to grasp new knowledge of state management. And also I am yet to deeply work with components to access how well we can manage state for components

Avuidrauxs commented 5 years ago

@atherdon , Yes let's create a small one task that will enable me to blend back easily to the codebase , I've been absent for too long

atherdon commented 5 years ago

@Avuidrauxs ok, will create a small task and then reply to your question

Avuidrauxs commented 5 years ago

@atherdon Okay I'm waiting patiently :-)

atherdon commented 5 years ago

@Avuidrauxs please check #171 task