-
Notifications
You must be signed in to change notification settings - Fork 351
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
Fix css issues with fullscreen where bottom bar was hidden #1467
Conversation
@@ -181,12 +181,13 @@ export const StyledStatusBarWrapper = styled.div` | |||
display: none; | |||
` | |||
|
|||
export const StyledStatusBar = styled.div` | |||
export const StyledStatusBar = styled.div<{ fullscreen?: boolean }>` |
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 is the bottom panel used in CypherFrame, when we're showing a graph visualization
export const StyledZoomHolder = styled.div` | ||
position: absolute; | ||
export const StyledZoomHolder = styled.div<{ fullscreen: boolean }>` | ||
position: ${props => (props.fullscreen ? 'fixed' : 'absolute')}; |
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.
The zoombuttons on the cypherframe
b9f860c
to
b8d9e06
Compare
@@ -123,7 +124,6 @@ export const StyledFrameContents = styled.div<FullscreenProps>` | |||
export const StyledFrameStatusbar = styled.div<FullscreenProps>` | |||
border-top: ${props => props.theme.inFrameBorder}; | |||
height: ${dim.frameStatusbarHeight - 1}px; | |||
${props => (props.fullscreen ? 'margin-top: -78px;' : '')}; |
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.
NICE if we can get rid of this :-D
These changes look good so far, but let's wait for the node properties panel work to progress further before merging this. (cc @jk05 ) |
73d426f
to
1fc2bde
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.
I noticed a small issue with the position of the expand toggle button (triangle in bottom right) when the status bar is multi-line, but it's not a big deal, and will get replaced anyway once we finish the node properties feature.
To solve the problem of the bottom bar moving of the screen when adding lines to a query in fullscreen a new flexbox was added to give the middle content 100% height and scroll on overflow, taking the available space not used for the editor or the bottom bar. For the main viz though, resizing the container seems to change the size of the nodes, so I've opted to use position fixed for the bottom bar and zoom controls there. Worth noting that the viz bottom bar is probably going away soon as part of our work on node properties. I'm sure there's a cleaner way to structure the css and element tree, so far this is the cleanest I've worked out after too many hours of fiddling.
preview @ http://fullscreen_fix.surge.sh
Fixes #1460