Skip to content

Commit

Permalink
Address PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
stratoula committed Nov 23, 2023
1 parent e229aa3 commit 6c8f8d7
Show file tree
Hide file tree
Showing 7 changed files with 90 additions and 77 deletions.
Expand Up @@ -36,7 +36,7 @@ export const textBasedLanguagedEditorStyles = (
border: isCompactFocused ? euiTheme.border.thin : 'none',
borderLeft: editorIsInline || !isCompactFocused ? 'none' : euiTheme.border.thin,
borderRight: editorIsInline || !isCompactFocused ? 'none' : euiTheme.border.thin,
borderTopLeftRadius: isCodeEditorExpanded ? 0 : '6px',
borderTopLeftRadius: isCodeEditorExpanded ? 0 : euiTheme.border.radius.medium,
borderBottom: isCodeEditorExpanded
? 'none'
: isCompactFocused
Expand All @@ -48,8 +48,8 @@ export const textBasedLanguagedEditorStyles = (
width: isCodeEditorExpanded ? '100%' : `calc(100% - ${hasReference ? 80 : 40}px)`,
alignItems: isCompactFocused ? 'flex-start' : 'center',
border: !isCompactFocused ? euiTheme.border.thin : 'none',
borderTopLeftRadius: '6px',
borderBottomLeftRadius: '6px',
borderTopLeftRadius: euiTheme.border.radius.medium,
borderBottomLeftRadius: euiTheme.border.radius.medium,
borderBottomWidth: hasErrors ? '2px' : '1px',
borderBottomColor: hasErrors ? euiTheme.colors.danger : euiTheme.colors.lightShade,
},
Expand Down Expand Up @@ -80,20 +80,20 @@ export const textBasedLanguagedEditorStyles = (
backgroundColor: euiTheme.colors.lightestShade,
paddingLeft: euiTheme.size.base,
paddingRight: euiTheme.size.base,
paddingTop: editorIsInline ? '7px' : euiTheme.size.xs,
paddingBottom: editorIsInline ? '7px' : euiTheme.size.xs,
paddingTop: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
paddingBottom: editorIsInline ? euiTheme.size.s : euiTheme.size.xs,
width: 'calc(100% + 2px)',
position: 'relative' as 'relative', // cast string to type 'relative',
marginTop: 0,
marginLeft: 0,
marginBottom: 0,
borderBottomLeftRadius: editorIsInline ? 0 : '6px',
borderBottomRightRadius: editorIsInline ? 0 : '6px',
borderBottomLeftRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
borderBottomRightRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
},
topContainer: {
border: editorIsInline ? 'none' : euiTheme.border.thin,
borderTopLeftRadius: editorIsInline ? 0 : '6px',
borderTopRightRadius: editorIsInline ? 0 : '6px',
borderTopLeftRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
borderTopRightRadius: editorIsInline ? 0 : euiTheme.border.radius.medium,
backgroundColor: euiTheme.colors.lightestShade,
paddingLeft: euiTheme.size.base,
paddingRight: euiTheme.size.base,
Expand Down
Expand Up @@ -31,6 +31,7 @@ export const FlyoutWrapper = ({
isScrollable,
displayFlyoutHeader,
language,
attributesChanged,
onCancel,
navigateToLensEditor,
onApply,
Expand All @@ -48,28 +49,29 @@ export const FlyoutWrapper = ({
>
<EuiFlexGroup justifyContent="spaceBetween" alignItems="center" responsive={false}>
<EuiFlexItem grow={false}>
<EuiFlexGroup alignItems="center" gutterSize="xs">
<EuiFlexItem grow={false}>
<EuiTitle size="xs">
<h2>
{i18n.translate('xpack.lens.config.editVisualizationLabel', {
defaultMessage: 'Edit {lang} visualization',
values: { lang: language },
})}
</h2>
</EuiTitle>
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiTitle size="xs">
<h2>
{i18n.translate('xpack.lens.config.editVisualizationLabel', {
defaultMessage: 'Edit {lang} visualization',
values: { lang: language },
})}
<EuiToolTip
content={i18n.translate('xpack.lens.config.experimentalLabel', {
defaultMessage:
'Technical preview, ES|QL currently offers limited configuration options',
})}
>
<EuiBetaBadge label="Lab" iconType="beaker" />
<EuiBetaBadge
label="Lab"
iconType="beaker"
size="s"
css={css`
margin-left: ${euiThemeVars.euiSizeXS};
`}
/>
</EuiToolTip>
</EuiFlexItem>
</EuiFlexGroup>
</h2>
</EuiTitle>
</EuiFlexItem>
{navigateToLensEditor && (
<EuiFlexItem grow={false}>
Expand Down Expand Up @@ -137,6 +139,7 @@ export const FlyoutWrapper = ({
aria-label={i18n.translate('xpack.lens.config.applyFlyoutAriaLabel', {
defaultMessage: 'Apply changes',
})}
disabled={!attributesChanged}
iconType="check"
data-test-subj="applyFlyoutButton"
>
Expand Down
Expand Up @@ -6,7 +6,7 @@
*/

import React, { useMemo } from 'react';
import { EuiSpacer, EuiFlexItem, useEuiTheme } from '@elastic/eui';
import { EuiSpacer, useEuiTheme } from '@elastic/eui';
import { css } from '@emotion/react';
import { VisualizationToolbar } from '../../../editor_frame_service/editor_frame/workspace_panel';
import { ConfigPanelWrapper } from '../../../editor_frame_service/editor_frame/config_panel/config_panel';
Expand All @@ -24,6 +24,7 @@ export function LayerConfiguration({
datasourceMap,
datasourceId,
framePublicAPI,
hasPadding,
setIsInlineFlyoutVisible,
}: LayerConfigurationProps) {
const dispatch = useLensDispatch();
Expand Down Expand Up @@ -59,9 +60,9 @@ export function LayerConfiguration({
setIsInlineFlyoutVisible,
};
return (
<EuiFlexItem
<div
css={css`
padding: ${euiTheme.size.s};
padding: ${hasPadding ? euiTheme.size.s : 0};
`}
>
<VisualizationToolbar
Expand All @@ -70,6 +71,6 @@ export function LayerConfiguration({
/>
<EuiSpacer size="m" />
<ConfigPanelWrapper {...layerPanelsProps} />
</EuiFlexItem>
</div>
);
}
Expand Up @@ -137,7 +137,7 @@ export function LensEditConfigurationFlyout({
]);

const onApply = useCallback(() => {
if (savedObjectId && attributesChanged) {
if (savedObjectId) {
const dsStates = Object.fromEntries(
Object.entries(datasourceStates).map(([id, ds]) => {
const dsState = ds.state;
Expand Down Expand Up @@ -171,7 +171,6 @@ export function LensEditConfigurationFlyout({
closeFlyout?.();
}, [
savedObjectId,
attributesChanged,
closeFlyout,
datasourceStates,
visualization.state,
Expand Down Expand Up @@ -235,6 +234,7 @@ export function LensEditConfigurationFlyout({
navigateToLensEditor={navigateToLensEditor}
onApply={onApply}
isScrollable={true}
attributesChanged={attributesChanged}
>
<LayerConfiguration
attributes={attributes}
Expand All @@ -243,6 +243,7 @@ export function LensEditConfigurationFlyout({
visualizationMap={visualizationMap}
datasourceMap={datasourceMap}
datasourceId={datasourceId}
hasPadding={true}
framePublicAPI={framePublicAPI}
setIsInlineFlyoutVisible={setIsInlineFlyoutVisible}
/>
Expand All @@ -258,6 +259,7 @@ export function LensEditConfigurationFlyout({
onCancel={onCancel}
navigateToLensEditor={navigateToLensEditor}
onApply={onApply}
attributesChanged={attributesChanged}
language={getLanguageDisplayName(textBasedMode)}
isScrollable={false}
>
Expand Down Expand Up @@ -327,13 +329,22 @@ export function LensEditConfigurationFlyout({
css={css`
padding-left: ${euiThemeVars.euiSize};
padding-right: ${euiThemeVars.euiSize};
.euiAccordion__childWrapper {
flex: ${isLayerAccordionOpen ? 1 : 'none'}
}
}
`}
>
<EuiAccordion
id="layer-configuration"
buttonContent={
<EuiTitle size="xxs">
<EuiTitle
size="xxs"
css={css`
padding: 2px;
}
`}
>
<h5>
{i18n.translate('xpack.lens.config.layerConfigurationLabel', {
defaultMessage: 'Layer configuration',
Expand All @@ -352,7 +363,6 @@ export function LensEditConfigurationFlyout({
}
setIsLayerAccordionOpen(!isLayerAccordionOpen);
}}
paddingSize="none"
>
<LayerConfiguration
attributes={attributes}
Expand All @@ -375,6 +385,9 @@ export function LensEditConfigurationFlyout({
border-bottom: ${euiThemeVars.euiBorderThin};
padding-left: ${euiThemeVars.euiSize};
padding-right: ${euiThemeVars.euiSize};
.euiAccordion__childWrapper {
flex: ${isSuggestionsAccordionOpen ? 1 : 'none'}
}
}
`}
>
Expand Down
Expand Up @@ -19,6 +19,7 @@ export interface FlyoutWrapperProps {
isScrollable: boolean;
displayFlyoutHeader?: boolean;
language?: string;
attributesChanged?: boolean;
onCancel?: () => void;
onApply?: () => void;
navigateToLensEditor?: () => void;
Expand Down Expand Up @@ -79,5 +80,6 @@ export interface LayerConfigurationProps {
datasourceMap: DatasourceMap;
datasourceId: 'formBased' | 'textBased';
framePublicAPI: FramePublicAPI;
hasPadding?: boolean;
setIsInlineFlyoutVisible: (flag: boolean) => void;
}
Expand Up @@ -394,6 +394,7 @@ export function LayerPanel(
)}
</EuiFlexGroup>
{props.indexPatternService &&
!isTextBasedLanguage &&
(layerDatasource || activeVisualization.LayerPanelComponent) && (
<EuiSpacer size="s" />
)}
Expand Down
Expand Up @@ -21,8 +21,7 @@ import {
EuiButtonEmpty,
EuiAccordion,
EuiText,
EuiFlexGroup,
EuiFlexItem,
EuiNotificationBadge,
} from '@elastic/eui';
import { euiThemeVars } from '@kbn/ui-theme';
import { IconType } from '@elastic/eui/src/components/icon/icon';
Expand Down Expand Up @@ -197,9 +196,7 @@ const SuggestionPreview = ({
css: css`
display: flex;
flex-direction: column;
flex-basis: calc(50% - ${euiThemeVars.euiSize});
margin-right: ${euiThemeVars.euiSize};
margin-bottom: ${euiThemeVars.euiSize};
flex-basis: calc(50% - 9px);
`,
}
: undefined
Expand Down Expand Up @@ -522,7 +519,13 @@ export function SuggestionPanel({
);
};
const title = (
<EuiTitle size="xxs">
<EuiTitle
size="xxs"
css={css`
padding: 2px;
}
`}
>
<h3>
<FormattedMessage
id="xpack.lens.editorFrame.suggestionPanelTitle"
Expand All @@ -531,25 +534,6 @@ export function SuggestionPanel({
</h3>
</EuiTitle>
);
const accordionWithSuggestionsCount = (
<EuiFlexGroup gutterSize="none" alignItems="center" justifyContent="spaceBetween">
<EuiFlexItem grow={false}>{title}</EuiFlexItem>
<EuiFlexItem grow={false}>
<div
css={css`
background-color: ${euiThemeVars.euiColorLightShade};
height: ${euiThemeVars.euiSizeL};
width: ${euiThemeVars.euiSizeL};
text-align: center;
border-radius: ${euiThemeVars.euiSizeXS};
`}
>
{suggestions.length + 1}
</div>
</EuiFlexItem>
</EuiFlexGroup>
);
const buttonContent = wrapSuggestions ? accordionWithSuggestionsCount : title;
return (
<EuiAccordion
id="lensSuggestionsPanel"
Expand All @@ -561,30 +545,38 @@ export function SuggestionPanel({
css={css`
padding-bottom: ${wrapSuggestions ? 0 : euiThemeVars.euiSizeS};
`}
buttonContent={buttonContent}
buttonContent={title}
forceState={hideSuggestions ? 'closed' : 'open'}
onToggle={toggleSuggestions}
extraAction={
existsStagedPreview &&
!hideSuggestions && (
<EuiToolTip
content={i18n.translate('xpack.lens.suggestion.refreshSuggestionTooltip', {
defaultMessage: 'Refresh the suggestions based on the selected visualization.',
})}
>
<EuiButtonEmpty
data-test-subj="lensSubmitSuggestion"
size="xs"
iconType="refresh"
onClick={() => {
dispatchLens(submitSuggestion());
}}
>
{i18n.translate('xpack.lens.sugegstion.refreshSuggestionLabel', {
defaultMessage: 'Refresh',
})}
</EuiButtonEmpty>
</EuiToolTip>
<>
{existsStagedPreview && (
<EuiToolTip
content={i18n.translate('xpack.lens.suggestion.refreshSuggestionTooltip', {
defaultMessage: 'Refresh the suggestions based on the selected visualization.',
})}
>
<EuiButtonEmpty
data-test-subj="lensSubmitSuggestion"
size="xs"
iconType="refresh"
onClick={() => {
dispatchLens(submitSuggestion());
}}
>
{i18n.translate('xpack.lens.sugegstion.refreshSuggestionLabel', {
defaultMessage: 'Refresh',
})}
</EuiButtonEmpty>
</EuiToolTip>
)}
{wrapSuggestions && (
<EuiNotificationBadge size="m" color="subdued">
{suggestions.length + 1}
</EuiNotificationBadge>
)}
</>
)
}
>
Expand All @@ -595,6 +587,7 @@ export function SuggestionPanel({
tabIndex={0}
css={css`
flex-wrap: ${wrapSuggestions ? 'wrap' : 'nowrap'};
gap: ${wrapSuggestions ? euiThemeVars.euiSize : 0};
`}
>
{changesApplied ? renderSuggestionsUI() : renderApplyChangesPrompt()}
Expand Down

0 comments on commit 6c8f8d7

Please sign in to comment.