-
-
Notifications
You must be signed in to change notification settings - Fork 379
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
serverInfo: show server info at MenuFooter #694
Conversation
@@ -7,7 +7,6 @@ import { HeaderActions } from './components/HeaderActions/HeaderActions'; | |||
import { Loader } from './components/Loader/Loader'; | |||
import { Menu } from './components/Menu/Menu'; | |||
import { Title } from './components/Title/Title'; | |||
import { useActiveQueue } from './hooks/useActiveQueue'; |
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.
We dont really need this here, each component that needs can fetch with its own hook
const { t } = useTranslation(); | ||
const { queues } = useQueues(); |
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.
also this way typescript infers the types and we dont need to import anything else
description?: string; | ||
} | ||
export const Title = () => { | ||
const queue = useActiveQueue(); |
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.
as hooks can be composed from other hooks, this hook doesnt need to have any arguments, it will resolve automatically which is the active queue
yeah that could work, what do you think about the prop drilling fixing it was also proposed here? |
Regarding prop drilling, it looks OK, |
sure mate :) |
This PR does 2 things (I can split this in two if you want) as I started thinking I could place the connection information next to the
Title
, so I refactored to avoid prop-drilling and each component to use exactly the info they need via hooks.Then realized that would be better placed at the side just above the version so its not getting that much attention, this is mainly to be 100% sure of where your app is connected, as many times you are switching between local/qa/prod so need to be extra careful sometimes on what you are doing.