Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ exports[`useDataGridStyleSelector styleSelector renders a toolbar button/popover
hasArrow={true}
isOpen={false}
ownFocus={true}
panelClassName="euiDataGrid__controlPopover"
panelClassName="euiDataGrid__stylePopoverPanel"
panelPaddingSize="s"
>
<EuiI18n
Expand Down
4 changes: 2 additions & 2 deletions src/components/datagrid/controls/_data_grid_display.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
.euiDataGrid__displayFormRow {
width: $euiFormMaxWidth;
.euiDataGrid__stylePopoverPanel {
Comment thread
cchaos marked this conversation as resolved.
Outdated
width: $euiFormMaxWidth + $euiSize;
}
11 changes: 1 addition & 10 deletions src/components/datagrid/controls/_data_grid_toolbar.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
position: relative;
z-index: 3; // Needs to sit above the content blow that sits below it
border: $euiBorderThin;
padding: $euiSizeXS;
padding: $euiSizeXS $euiSizeXS $euiSizeXS 0;
display: flex;
justify-content: space-between;
}
Expand All @@ -28,18 +28,9 @@
}
}

.euiDataGrid__controlBtn {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Fantastic, thanks for cleaning this up!

border-radius: $euiBorderRadius;

&:focus {
background: shadeOrTint($euiColorLightestShade, 10%, 10%);
}
}

.euiDataGrid__controlBtn--active,
.euiDataGrid__controlBtn--active:focus {
font-weight: $euiFontWeightSemiBold;
color: $euiColorFullShade;
}

@include euiDataGridStyles(bordersNone) {
Expand Down
19 changes: 13 additions & 6 deletions src/components/datagrid/controls/data_grid_toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* Side Public License, v 1.
*/

import React from 'react';
import React, { useEffect } from 'react';
import { EuiButtonIcon } from '../../button';
import { useEuiI18n } from '../../i18n';
import {
Expand Down Expand Up @@ -48,11 +48,18 @@ export const EuiDataGridToolbar = ({
? true
: gridWidth > minSizeForControls || isFullScreen;

// need to safely access those Web APIs
if (typeof document !== 'undefined') {
// When data grid is full screen, we add a class to the body to remove the extra scrollbar
document.body.classList.toggle(GRID_IS_FULLSCREEN_CLASSNAME, isFullScreen);
}
useEffect(() => {
// When data grid is full screen, we add a class to the body to remove the extra scrollbar and stay above any fixed headers
if (isFullScreen && typeof document !== 'undefined') {
document.body.classList.add(GRID_IS_FULLSCREEN_CLASSNAME);

return () => {
if (typeof document !== 'undefined') {
document.body.classList.remove(GRID_IS_FULLSCREEN_CLASSNAME);
}
};
}
}, [isFullScreen]);
Comment on lines +52 to +63

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Super awesome fix, thanks! Do you think this needs a changelog entry or probably not?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Wouldn't hurt to give it a callout

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

And I think that actually closes this: elastic#5092

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Nice, I added a changelog entry and a closes commit (which I think should auto close on merge to main?)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I honestly don't know if a commit message will. But might as well add it to your PR summary as well.


const fullScreenSelector = (
<EuiButtonIcon
Expand Down
2 changes: 1 addition & 1 deletion src/components/datagrid/controls/style_selector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const useDataGridStyleSelector = (
closePopover={() => setIsOpen(false)}
anchorPosition="downRight"
panelPaddingSize="s"
panelClassName="euiDataGrid__controlPopover"
panelClassName="euiDataGrid__stylePopoverPanel"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This still needs the generic .euiDataGrid__controlPopover class, as that CSS has transform/z-index stuff specific to all toolbar popovers

Suggested change
panelClassName="euiDataGrid__stylePopoverPanel"
panelClassName="euiDataGrid__controlPopover euiDataGrid__stylePopoverPanel"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That transform stuff is actually only needed on popovers whose contents contains the drag-and-drop functionality like the sort and column selector. This new popover is just a simple form.

@cee-chen cee-chen Nov 8, 2021

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Ohhh that's super good to know! Now I feel like I should have named that CSS class better. I'll fix that after this merges

button={
<EuiButtonIcon
size="xs"
Expand Down
12 changes: 6 additions & 6 deletions src/components/datagrid/data_grid_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ export type CommonGridProps = CommonProps &
*/
gridStyle?: EuiDataGridStyle;
/**
* Accepts either a boolean or #EuiDataGridToolbarVisibilityOptions object. When used as a boolean, defines the display of the toolbar entire. WHen passed an object allows you to turn off individual controls within the toolbar as well as add additional buttons.
* Accepts either a boolean or #EuiDataGridToolBarVisibilityOptions object. When used as a boolean, defines the display of the toolbar entire. WHen passed an object allows you to turn off individual controls within the toolbar as well as add additional buttons.
*/
toolbarVisibility?: boolean | EuiDataGridToolBarVisibilityOptions;
/**
Expand Down Expand Up @@ -609,20 +609,20 @@ export interface EuiDataGridToolBarVisibilityOptions {
showFullScreenSelector?: boolean;
/**
* If passed a `ReactNode`, appends the passed custom control into the left side of the toolbar, after the column & sort controls.
* Use #EuiDataGridToolBarAdditionalControlsOptions to customize the location of your control.
* Or use #EuiDataGridToolBarAdditionalControlsOptions to customize the location of your control.
*/
additionalControls?: ReactNode | EuiDataGridToolBarAdditionalControlsOptions;
}

export interface EuiDataGridToolBarAdditionalControlsOptions {
/**
* Will append the passed node into the left side of the toolbar, after the column & sort controls.
* We recommend using `xs`-sized `EuiButtonEmpty`s to match the existing controls on the left.
* Will append the passed node into the left side of the toolbar, **after** the column & sort controls.
* We recommend using `<EuiButtonEmpty size="xs" />` to match the existing controls on the left.
*/
left?: ReactNode;
/**
* Will prepend the passed node into the right side of the toolbar, before the density & full screen controls.
* We recommend using `xs`-sized `EuiButtonIcons`s to match the existing controls on the right.
* Will prepend the passed node into the right side of the toolbar, **before** the density & full screen controls.
* We recommend using `<EuiButtonIcon size="xs" />` to match the existing controls on the right.
*/
right?: ReactNode;
}
Expand Down