-
Notifications
You must be signed in to change notification settings - Fork 195
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
frontend: Add detail drawer mode #1447
base: main
Are you sure you want to change the base?
Conversation
d115bb2
to
3f7e7c1
Compare
Commit Notes
|
3f7e7c1
to
64cb1e2
Compare
Note
|
64cb1e2
to
b49bdef
Compare
note
|
b49bdef
to
e828312
Compare
Work note Progress
Issues
|
e828312
to
49e7160
Compare
Work note Progress
Issues
|
49e7160
to
b2c9f0b
Compare
Commit fix pushed
|
b2c9f0b
to
5e443f0
Compare
After a considerable amount of time spent on working in a closed drawer version (a squished column to the right of the screen) I think the better solution would be to use a floating drawer open button which has fixed a number of issues related to the original squished drawer idea Fixes
Issues
Solved |
5e443f0
to
e313b44
Compare
Solved
|
63f5c73
to
3bd3c63
Compare
Solved
|
ISSUE
|
Some meeting notes...
experiments in drawer-details2 branch The next step is to change the Link inside frontend/src/components/common/Link.tsx to when drawer setting is turned on to produce links like Add a method to get the searchParams for a resource in headlamp/frontend/src/lib/k8s/cluster.ts something like: getListPlusDetails() {
// return a link like this: /c/minikube/pods?name=coredns-xkjxckvjxkcjv&namespace=kube-system
// returns listRoute + ?name=&namespace=
} |
3bd3c63
to
e3ddd00
Compare
33bbf31
to
c3df513
Compare
last push: I think this fixes the typescript error for kubeObject & void return setting the slice state for the current resource to be just the json part of the kubeObject being set (we were extracting just the json part anyway to render in the drawer from the selectedResource state (DetailsDrawer.tsx line 9) DetailsDrawerLink |
ea8417c
to
d96ab40
Compare
@sniok review ready |
|
||
// DETAIL DRAWER MODE | ||
const isDetailDrawerEnabled = useTypedSelector(state => state.drawerMode.isDetailDrawerEnabled); | ||
const [isDrawerEnabled, changeDetailDrawerEnabled] = useState<any>(isDetailDrawerEnabled); |
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.
isDrawerEnabled is duplicated state, use only the state from the store instead
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.
done!
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 moved the initial value inside the useState which wasn't my point. I mean that useState is not needed here, you can use state.drawerMode.isDetailDrawerEnabled
directly by reading from redux and setting value directly to redux.
Then you won't have duplicated state and won't need useEffect on line 25 that syncs value to the redux store and dispatches on line 18-22
const isDrawerEnabled = useTypedSelector(state => state.drawerMode.isDetailDrawerEnabled);
const setIsDrawerEnabled = (isEnabled: boolean) => dispatch(setDetailDrawerEnabled(isEnabled));
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.
done!
d96ab40
to
68644ca
Compare
@@ -53,8 +53,9 @@ | |||
"Delete Plugin": "", | |||
"Are you sure you want to delete this plugin?": "", | |||
"Save": "", | |||
"Uh-oh! Something went wrong.": "Oh-oh! Etwas ist schief gelaufen.", | |||
"Uh-oh! Something went wrong.": "", |
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.
this translation is gone
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.
running i18n removes it
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.
done!
<Box width={800}> | ||
<Box style={{ marginTop: '5rem', marginBottom: '2rem' }}> | ||
<Button variant="outlined" color="primary" onClick={() => closeDrawer()}> | ||
Close |
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.
this string is not translated
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.
done!
|
||
// NOTICE THAT WE DO NOT USE isDrawerEnabled TO DETERMINE HOW THE SWITCH IS RENDERED UNDER THE CHECKED PROP, THIS IS BECAUSE THE USEEFFECT WILL RERENDER THE COMPONENT WITH THE NEW STATE | ||
return ( | ||
<FormControlLabel |
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.
I think this setting needs some explanation for what "Drawer mode" is. Just a Switch with label "drawer mode" won't be clear to everyone
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.
added a tooltip explanation
93d716c
to
d08eeda
Compare
added description to drawer mode button fixed drawer not working on app mode (currently only updating the URL for browser mode) |
d08eeda
to
0eb2776
Compare
|
||
// DETAIL DRAWER MODE | ||
const isDetailDrawerEnabled = useTypedSelector(state => state.drawerMode.isDetailDrawerEnabled); | ||
const [isDrawerEnabled, changeDetailDrawerEnabled] = useState<any>(isDetailDrawerEnabled); |
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 you also try to split the commit into separate ones? For example one with adding the slice, another with settings, another with updates to the Link, etc |
af8bdbb
to
aaed78e
Compare
Signed-off-by: Vincent T <[email protected]>
Signed-off-by: Vincent T <[email protected]>
Signed-off-by: Vincent T <[email protected]>
Signed-off-by: Vincent T <[email protected]>
Signed-off-by: Vincent T <[email protected]>
Signed-off-by: Vincent T <[email protected]>
8bd6eed
to
aab4894
Compare
Introduce Detail Drawer Mode in Headlamp
For issue #1354
Description
This PR introduces a new 'Detail Drawer' mode to Headlamp, providing users with the option to view details in a side drawer. This enhances the user interface by allowing for a more flexible and efficient way of displaying detailed information without navigating away from the current view.
Features
Verification
WIP
Progress
Issues