-
Notifications
You must be signed in to change notification settings - Fork 11
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
Issue 212 frontend query sql #221
Conversation
Visit the preview URL for this PR (updated for commit 00ad14a): https://codeclimbersio--pr221-issue-212-frontend-q-3pfssudm.web.app (expires Sat, 28 Sep 2024 00:19:21 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 3c28ab1d661a7526dc2530b2448a1aa4f88d23d4 |
0bdbe9f
to
24444e5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your code is always so clean. Looks really good man 🔥 .
Honestly I use to hate classes after everywhere I've worked using them poorly, but you're changing my mind!
target?: string | ||
} | ||
|
||
const CodeClimbersLoadingButton = ({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it just be better to have a generic button component that takes in the eventName? LoadingButton to me mean that you'd be displaying a spinner and expect a isLoading | isPending
prop
|
||
function DashboardLayout({ children }: DashboardLayoutProps) { | ||
const { isMajorUpdate, isMinorUpdate } = useUpdateVersionHook() | ||
if (isMajorUpdate || isMinorUpdate) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Smart!
@@ -28,7 +29,20 @@ export async function bootstrap() { | |||
}) | |||
traceEnvironment() | |||
|
|||
app.enableCors() | |||
app.enableCors({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice
credentials: 'include', | ||
}) | ||
return useBetterQuery<T[], Error>({ | ||
queryKey: ['localdb/query'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You'll want to make this key unique per query string passed in. As is it'll reuse the same key for any query string passed in
The Pull Request is ready
issue-123-enable-x-does-not-disable-y
Changes
The code follows best practices
any
type usedNotes