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

feat: auth screens #8

Merged
merged 20 commits into from
Jul 22, 2024
Merged

feat: auth screens #8

merged 20 commits into from
Jul 22, 2024

Conversation

typeWolffo
Copy link
Member

@typeWolffo typeWolffo commented Jul 16, 2024

image
image
image
image
image

@typeWolffo typeWolffo requested review from mikoscz and k1eu July 16, 2024 21:54
@typeWolffo typeWolffo self-assigned this Jul 16, 2024
@typeWolffo typeWolffo changed the title feat: auth screens [WIP]feat: auth screens Jul 17, 2024
Comment on lines 19 to 23
app.enableCors({
origin: "https://app.guidebook.localhost",
credentials: true,
});

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

origin should come from envs so it's usable when deployed

"morgan": "^1.10.0",
"next-themes": "^0.3.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the next-themes for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed

Comment on lines 28 to 33
<Sheet open={isSheetOpen} onOpenChange={setIsSheetOpen}>
<SheetContent side="left">
<SheetHeader>
<SheetDescription>
<div className="flex flex-col gap-2 px-2">
<div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the Sheet itself can be a seprate compoent

Comment on lines 30 to 32
document.documentElement.classList.toggle("dark");
localStorage.setItem(
"theme",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can add a listerner somewhere on top so it also renders correct mode after refresh

Copy link
Member Author

@typeWolffo typeWolffo Jul 19, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@k1eu so I made the theme toggle completely new using the zustand store. What do you think?
(useTheme from shadcn doesn't seem to support SPA mode in the remix)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 🗡️

Base automatically changed from jw/user-auth to main July 18, 2024 12:13
@typeWolffo typeWolffo requested a review from k1eu July 19, 2024 14:12
@typeWolffo typeWolffo changed the title [WIP]feat: auth screens feat: auth screens Jul 19, 2024
Copy link
Member

@k1eu k1eu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • typebox instead of zod
  • 401 not making refresh calls when logged out
  • selector in zustand

@typeWolffo typeWolffo merged commit 59d4443 into main Jul 22, 2024
@typeWolffo typeWolffo deleted the jw/auth-screens branch July 22, 2024 17:16
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.

2 participants