-
Notifications
You must be signed in to change notification settings - Fork 37
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
Redirect to asset page if no installation selected #2049
Redirect to asset page if no installation selected #2049
Conversation
Closes #2042 |
1acbaa2
to
1adfbd5
Compare
1adfbd5
to
a542486
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.
Looks good otherwise
frontend/src/utils/ValidateURL.tsx
Outdated
import { config } from 'config' | ||
|
||
export function validateURL() { | ||
const { installationCode, installationName } = useInstallationContext() |
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.
Does using contexts inside non-component functions not cause an error/warning in the frontend?
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.
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.
Could we rename the function so that it's clear that it will navigate away, and not just validate the inspection?
a542486
to
414b8cc
Compare
414b8cc
to
3a7a86b
Compare
@@ -15,6 +16,8 @@ const StyledFrontPage = styled.div` | |||
` | |||
|
|||
export const FrontPage = () => { | |||
redirectIfNoInstallationSelected() |
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.
If we wanted to be even more optimal we could useMemo, or a useEffect with empty conditionals here, to prevent this function from needing to be rerun on each render, but it's not too important.
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.
Looks good otherwise. Maybe consider prevent the function from being rerun too often, but otherwise ready to merge.
Ready for review checklist: