mubaris / curiosity

Find Amazing Github :octocat: Projects :zap:
https://mubaris.github.io/curiosity/
244 stars 65 forks source link

Username Add/Remove issues #51

Closed mubaris closed 6 years ago

mubaris commented 7 years ago

Add new username in Settings button is not working. If I try to add a new username, it says Username not found.

CC: @maiquynhtruong

maiquynhtruong commented 7 years ago

Your username has to be valid. I did put a check to see if github.com/username is a valid link. Why don't you request changes on my PR? No need to make an issue haha

mubaris commented 7 years ago

We already merged those PRs. That's the reason for the issue. I think it's due to limit of API requests. You are checking for the validity of username before adding it. You need to include the Token in there. That's all.

maiquynhtruong commented 7 years ago

if your token is above the API request limit, shouldn't you need to get a new one?

mubaris commented 7 years ago

For checking, we are calling API without token.

Example:- api.github.com/users/mubaris

We need to change to something like this

api.github.com/users/mubaris?access_token=${tokenVar}

mubaris commented 7 years ago

I noted one more thing. Username array is still not persistent. You can add/remove usernames, but it just goes back to normal when you refresh.

Chris-Coffman commented 7 years ago

Should the username array be stored differently to allow for long term modification of user preferences? Perhaps storing the array in a cookie after the user sets their api token.

mubaris commented 7 years ago

We want to store this username array in localStorage

Chris-Coffman commented 7 years ago

But I take it you're ok will removing it from the current const array?

mubaris commented 7 years ago

The current const array is the default array. It's the fallback. If we are storing it in localStorage user can play with it. And if the user removes it from localStorage, we will reset localStorage from const array.

Chris-Coffman commented 7 years ago

OK, I see. What if the user wants to remove one of the defaults? Or are you content that if the users removes a default it will never be remembered after the session ends?

Either way I've committed the change to the user verification so that it now uses the provided API token and it now adds that user and the list re-renders just fine.

mubaris commented 7 years ago

User can remove elements of the default array, but user cannot remove the array from localStorage. If the user does that, we will restore it from default const array.

maiquynhtruong commented 7 years ago

@mubaris I think I know why it didn't run on yours. You might be accessing the web app through https://curiositylab.github.io/curiosity/. I tried that one too and it didn't save the username either.

Anyway I've added token to api calls and remove the line localStorage.setItem('usernames', JSON.stringify(USERNAMES)); at if (accessToken). That line resets the username array in localStorage even though it shouldn't happen. Hope it works this time.