search

the-difference-engine / ksf

7 stars 1 forks source link

at undefined field on click nomination problem #343

Closed somersbmatthews closed 3 years ago

somersbmatthews commented 3 years ago

Zenhub Link:

https://app.zenhub.com/workspaces/ksf-5f23520d91f7ee00134cbaf6/issues/the-difference-engine/ksf/272

Describe the problem being solved:

avoiding findAllNominations after clicking on a single link to a single nomination

Impacted areas in the application:

NominationPage

Describe the steps you took to test your changes:

npm run start

If this ticket involves any UI or email changes, please provide a screenshot that shows the updated UI

List general components of the application that this PR will affect: NominationPage PR checklist

somersbmatthews commented 3 years ago

Whenever I add the "findAllNominations" function to the array in useContext() then the useContext() doesn't work properly anywhere else (like in NominationPage)

vhoof commented 3 years ago

I'm not sure exactly how useContext works under the hood, but I'd expect if you return three items in the value, you may need to pull all three back out. You could return a different data structure, potentially, if you wanted to return a collection but only use an arbitrary subset on different pages.

Separately, maybe it'd be more appropriate to call findAllNominations on pages where we need to do that, and have a separate function to fetch/store a separate nomination where that's appropriate. @aschey Is this what you had in mind?

aschey commented 3 years ago

I'm not sure exactly how useContext works under the hood, but I'd expect if you return three items in the value, you may need to pull all three back out. You could return a different data structure, potentially, if you wanted to return a collection but only use an arbitrary subset on different pages.

Separately, maybe it'd be more appropriate to call findAllNominations on pages where we need to do that, and have a separate function to fetch/store a separate nomination where that's appropriate. @aschey Is this what you had in mind?

Yep, I think that's the end goal. We looked at this a bit on Monday and the number of items in the array wasn't the only issue so it seems like there's something else going on too.