Andrewnt219 / rent-near-me

https://rent-near-me.vercel.app/
1 stars 1 forks source link

Allow UserMenuLink to be button as well (not just link) #34

Closed vitokhangnguyen closed 3 years ago

vitokhangnguyen commented 3 years ago

Opening the Login or Register modal from the user menu requires the UserMenuLink to be a button (with onClick attribute)

Andrewnt219 commented 3 years ago

I think the login and register modal should be triggered by URL. That way, we can redirect and if users go to /register, they can see the modal.

vitokhangnguyen commented 3 years ago

I currently have the hook useLayoutModal to allow triggering the modal from anywhere already. Having a URL to trigger a modal is kinda unintuitive imo.

Andrewnt219 commented 3 years ago

Will we have any problems with redirecting unauthenticated users? It's the same experience for users, they just click a button and the modal shows up.

vitokhangnguyen commented 3 years ago

For my setup right now, it doesn't make a difference for redirecting unauthenticated user. Instead of doing router.redirect() (or something like that), we do...

const { loginModal } = useLayoutModal();
// ...

loginModal.show() // or .toggle()

Preview #30 for more info on this.

Andrewnt219 commented 3 years ago

In the scenario where a user navigates directly to his dashboard from bookmark, what the page in the background will be?

vitokhangnguyen commented 3 years ago

Good point. Maybe the homepage? But the problem is not eclusive to the way I did. If the user go to /register by bookmark, what do you think should be the background?

Andrewnt219 commented 3 years ago

Right. Airbnb said f the users and moved on. Homepage in the background sounds reasonable.