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.

Sign up for GitHub

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

Jump to bottom

Bugfix: Add proper padding when scrolling is added to bar gauge #32411

Closed
wants to merge 12 commits into from
Closed

Bugfix: Add proper padding when scrolling is added to bar gauge #32411

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
5e9e047
fixing so we get the scrollbar in the proper place for the vizualizat…
mckn Mar 29, 2021
6df7ac7
changed so the select is used with a portal.
mckn Mar 29, 2021
5a7aa39
increasing the zIndex of the selector.
mckn Mar 31, 2021
f493ec8
Merge remote-tracking branch 'origin/master' into mckn/bar-gauge-scroll
torkelo Apr 6, 2021
1746025
Testing custom scrollbar
torkelo Apr 6, 2021
380eda8
Merge branch 'mckn/bar-gauge-scroll' of github.com:grafana/grafana in…
torkelo Apr 6, 2021
54c3681
Progress but now DashList has no right padding
torkelo Apr 6, 2021
faed5ca
added back className
torkelo Apr 7, 2021
aaf8e39
wip
mckn Apr 7, 2021
24d9eef
fixing the issues with zIndexes and having the select as a portal.
mckn Apr 7, 2021
ea3ac70
adding portal to theme definition.
mckn Apr 7, 2021
d7810fc
test(selectbase): migrate tests to RTL and fix for portal related cha…
jackw Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions packages/grafana-data/src/panel/PanelPlugin.ts
View file
Open in desktop
Expand Up @@ -106,6 +106,7 @@ export class PanelPlugin<
onPanelMigration?: PanelMigrationHandler<TOptions>;
onPanelTypeChanged?: PanelTypeChangedHandler<TOptions>;
noPadding?: boolean;
scrollableY?: boolean;

/**
* Legacy angular ctrl. If this exists it will be used instead of the panel
Expand Down Expand Up @@ -199,6 +200,11 @@ export class PanelPlugin<
return this;
}

setScrollable(scrollableY = false) {
this.scrollableY = scrollableY;
return this;
}

/**
* This function is called before the panel first loads if
* the current version is different than the version that was saved.
Expand Down
1 change: 0 additions & 1 deletion packages/grafana-ui/src/components/VizRepeater/VizRepeater.tsx
View file
Open in desktop
Expand Up @@ -152,7 +152,6 @@ export class VizRepeater<V, D = {}> extends PureComponent<Props<V, D>, State<V>>

const repeaterStyle: React.CSSProperties = {
display: 'flex',
overflow: minVizHeight ? 'hidden auto' : 'hidden',
mckn marked this conversation as resolved.
Show resolved Hide resolved
};

let vizHeight = height;
Expand Down
4 changes: 3 additions & 1 deletion public/app/features/dashboard/dashgrid/PanelChrome.tsx
View file
Open in desktop
Expand Up @@ -274,10 +274,12 @@ export class PanelChrome extends Component<Props, State> {
const headerHeight = this.hasOverlayHeader() ? 0 : theme.panelHeaderHeight;
const chromePadding = plugin.noPadding ? 0 : theme.panelPadding;
const panelWidth = width - chromePadding * 2 - PANEL_BORDER;
const innerPanelHeight = height - headerHeight - chromePadding * 2 - PANEL_BORDER;
const scrollableYMargin = plugin.scrollableY ? theme.panelPadding : 0;
const innerPanelHeight = height - headerHeight - chromePadding * 2 - PANEL_BORDER - scrollableYMargin;
const panelContentClassNames = classNames({
'panel-content': true,
'panel-content--no-padding': plugin.noPadding,
'panel-content--scrollable-y': plugin.scrollableY,
Copy link
Member

Choose a reason for hiding this comment

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

Not sure we need this special logic. and If plugin.scrollable is true we should wrap the component in a CustomScrollbar

});
const panelOptions = panel.getOptions();

Expand Down
1 change: 1 addition & 0 deletions public/app/plugins/panel/bargauge/module.tsx
View file
Open in desktop
Expand Up @@ -7,6 +7,7 @@ import { barGaugePanelMigrationHandler } from './BarGaugeMigrations';

export const plugin = new PanelPlugin<BarGaugeOptions>(BarGaugePanel)
.useFieldConfig()
.setScrollable(true)
.setPanelOptions((builder) => {
addStandardDataReduceOptions(builder);
addOrientationOption(builder);
Expand Down
11 changes: 11 additions & 0 deletions public/sass/pages/_dashboard.scss
View file
Open in desktop
Expand Up @@ -102,11 +102,22 @@ div.flot-text {
width: 100%;
flex-grow: 1;
height: calc(100% - #{$panel-header-height});
overflow: hidden;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: this feels a bit dangerous. Do you know about any use cases that might introduce a bug by adding this code? @torkelo

Copy link
Member

Choose a reason for hiding this comment

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

it breaks the dropdown select in Table panel, if we had a proper portal for Select component that could solve that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, damn...so I need to rethink this a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@torkelo I changed so we use the portal functionality of react-select...not sure if this is the best way to proceed but I didn't need to do so much work to get it in place.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me know if you think this is something we should proceed with. I will wait with fixing the tests until I got a 👍🏻

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And I guess we want to support horizontal scrolling as well.


&--no-padding {
padding: 0;
}

&--scrollable-y {
overflow: hidden auto;
margin-bottom: $panel-padding;
}

&--no-padding.panel-content--scrollable-y {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: can I write this in some better way using sass?

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
&--no-padding.panel-content--scrollable-y {
&--no-padding#{&}--scrollable-y {

might do the trick :)

margin-bottom: 0;
}
}

.dashboard-header {
font-size: $font-size-h2;
text-align: center;
Expand Down