Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If not logged in, login Alert is not pinned on homepage #49

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DhairyaBahl
Copy link
Contributor

Fixes #16

If not logged in, you shouldn't get pinned to the homepage.

@DhairyaBahl
Copy link
Contributor Author

DhairyaBahl commented Mar 15, 2022

@krancour Facing a small problem right now. In the logged out state, when I simply open the / route and then navigate to some other route, then the alert is still not shown. I am unable to detect the route change for the react component. There are many workarounds to do so but I am not sure which one is perfect. One simple way is to make the component re-render on route change but that would be quite heavy. Still trying to find the perfect way to do so.

Kindly tell me what is the perfect way and I will work on that asap. Thanks :)

Edit

According to me, best way is to simply convert the component to a functional component and we can then simply use the useLocation Hook or if we want to achieve the same result with the existing class component, then we can simply use the concept of higherOrderComponent with the useLocation hook like its shown over here

@krancour
Copy link
Contributor

Let's put closure on #48 before we go too deep down this rabbit hole. I imagine that where we land on #48 may affect what our options are here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If not logged in, you shouldn't get pinned to the homepage
2 participants