Open wise-introvert opened 3 years ago
This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.
🔍 Inspect: https://vercel.com/stephin007/cowin-vaccine-availablity-checker/2PMBv7X4TkcqxdCDJ9c27yyghShQ
✅ Preview: https://cowin-vaccine-availablity-checker-git-fix-issue-122-stephin007.vercel.app
hey @wise-introvert few points:
Point 1: When search criteria is search by district, when we change the state, the district does not change according to the changed state. But initially when we set the state it changes the district but when we change it, the district doesn't change.
Point 2: Say for example, user is in "search by district" , user faces the above situation and tries to change the search criteria to pincode and then goes back to search by district, the state still remains that means the user still faces the same issue as above.
Point 3: In vaccineDataSingle.js in the custom badge, you are sending a prop of background, which has a path "/covi/gi.test(vaccine?.vaccine)" i am not able to find any reference to this, it would be great if you could tell me what is this?
Point 4: " ...rest ", the use of this?(This is present in the index.js inside badge folder)
Point 5: In vaccineDataSingle.js , Opening and closing time is hidden at larger screens, any specific reason for that.
Point 6: In Home.js , in line 191 it says " if(true) ", what is the condition here? i mean if what is true, that the next piece of code will work.
Point 7: Just needed an general over view of the pages/home/index.js, Right now, is our home page rendering from this component?
Also, please remove all the unused variables, as this wont let the deployment to happen.
@stephin007
Point 1
has been resolved.vaccine?.vaccine
matches the regular expression /covi/gi
rest
in this case ) and then simply use it inside the component. Learn morepages/home/index.js
is being rendered as /home-page
in the browser. I did this to avoid any conflicts with the existing routes. Once the staging deployment of this PR is reviewed, I can switch to the actual route ( /vaccines
, I think ).Also, please remove all the unused variables, as this wont let the deployment to happen.
@stephin007 Is there any way you can disable the react-hooks warnings? Adding state methods to useEffect
dependency array will result in an indefinite loop, crashing the application.
Also, please remove all the unused variables, as this wont let the deployment to happen.
@stephin007 Is there any way you can disable the react-hooks warnings? Adding state methods to
useEffect
dependency array will result in an indefinite loop, crashing the application.
@wise-introvert we can just paste // eslint-disable
before the line where the warning is shown ..
and please resolve the conflicts!
@wise-introvert can you add the latest changes done in the new-ui branch!
These are some changes which i saw on UI, i am reviewing the rest of changes!
Okay, I'll sync this, feature branch with the NEW-UI
branch
Hey @wise-introvert , Regarding point 4 : i know about spread operator :P , i was asking about the word rest here , was this declared somewhere or does it just mean to send all the rest of the props? Can we some different words like "other" as it was given in the article.
rest
is like "i" and "j" are to for loop. It's what's usually used to denote "rest" of the props a component can receive. It can easily be changed though 😀
No No @wise-introvert , dont change just wanted to understand that! :)
hey @wise-introvert i hope you will be fixing the conflicts so that we merge to NEW-UI
Definitely. Sorry for the delay
Awesome @wise-introvert , i have mentioned you in some comments it would be great if you could address it as well.
Major Changes
VaccineDataSingle
component to be more responsive and scalable/src/lib/useClasses.js
Minor Changes