Skip to content
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

PanelEdit: Fixes resize pane border and spacing issues #56190

Closed
wants to merge 3 commits into from

Conversation

torkelo
Copy link
Member

@torkelo torkelo commented Oct 3, 2022

#54420 broke 3 things in panel edit (2 fixed so far in this PR) it also duplicated the resizer styling and the callbacks so that it's now something we have to maintain in two places (SplitView and SplitViewWrapper), that does not feel ideal.

  • Fixes border bug
  • Fixes a spacing bug (left split pane had overflow overflowY added which caused space for a scrollbar which caused unaligned spacing for the visualzation view in pane ledit)
  • Changing size of split pane is not saved like before, reloading panel edit rests it to the default. In 9.1 it still works and it remembers the split size

Below you see the border (should not be there), and the left side space is not equal to right side (should be 16px spacing between panes but it's 24)
image

@torkelo torkelo requested review from a team, axelavargas and kaydelaney and removed request for a team October 3, 2022 14:44
@grafanabot
Copy link
Contributor

Comment on lines -164 to -182
resizerV: cx(
resizer,
css`
cursor: col-resize;
width: ${paneSpacing};

&::before {
border-right: 1px solid transparent;
height: 100%;
left: 50%;
transform: translateX(-50%);
}

&::after {
height: 200px;
width: 4px;
}
`
),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think adding this back and removing SplitView and figuring out how to make SplitPaneWrapper more generic might be one path forward? or creating a standalone VerticalSplitView for explore

Copy link
Contributor

@gelicia gelicia Oct 3, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ox's thought was that a vertical split view was the core component, and dashboard edit which has vertical and horizontal would be in features/dashboard. Then both explore and dashboards use the core vertical split view.

@@ -83,7 +83,6 @@ export const SplitView = ({ uiState: { rightPaneSize }, children, minSize = 200,
minSize={minSize}
maxSize={width - minSize}
resizerClassName={useStyles2(getResizerStyles(hasSplit))}
paneStyle={{ overflow: 'auto', display: 'flex', flexDirection: 'column', overflowY: 'scroll' }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this overflowY is causing double right side spacing in Explore as well (and a lot of unnecessary scroll containers, as Explore.tsx already has overflow: scroll in it's container style)

Example of explore right side padding is 2x what it should be
Screenshot from 2022-10-03 17-23-49

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created #56213 to work on this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gelicia please help in this PR, needs to be fixed asap so we can backport this in 9.2 stable to we don't ship 9.2 with these bugs in panel edit and explore

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@torkelo The team discussed this and decided the best bet is to revert the split resize PRs so we can work on this outside of the release. We will be doing that shortly.

@torkelo torkelo added this to the 9.2.0 milestone Oct 4, 2022
@torkelo torkelo added the backport v9.2.x Mark PR for automatic backport to v9.2.x label Oct 4, 2022
@torkelo torkelo changed the title SplitPane: Fix border that should not be there PanelEdit: Fixes resize pane border and spacing issues Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants