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

[Observability] fix slo observability compressed styles to be not compressed #193081

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

[Observability] fix slo observability compressed styles to be not compressed #193081

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2fb8a5a
fix slo observability compressed styles to be not compressed
rshen91 Sep 16, 2024
67ea6a2
remove sass changes from controls plugin
rshen91 Sep 16, 2024
cf01d44
update
rshen91 Sep 16, 2024
c9d3ffe
fix
rshen91 Sep 17, 2024
0b6fbc0
Merge remote-tracking branch 'upstream/main' into slo-non-compressed
rshen91 Sep 17, 2024
8c45a2a
code review
rshen91 Sep 18, 2024
5354eb0
Merge remote-tracking branch 'upstream/main' into slo-non-compressed
rshen91 Sep 18, 2024
e590230
Merge remote-tracking branch 'upstream/main' into slo-non-compressed
rshen91 Sep 20, 2024
caa0bf2
code review sass clean up
rshen91 Sep 20, 2024
7f0d2f3
Merge remote-tracking branch 'origin/slo-non-compressed' into slo-non…
rshen91 Sep 20, 2024
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
Prev Previous commit
Next Next commit
update
  • Loading branch information
@rshen91
rshen91 committed Sep 16, 2024
commit cf01d44059d8d57ff5200381e402546c0f99c8ec
3 changes: 0 additions & 3 deletions src/plugins/controls/public/control_group/external_api/control_group_renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ export interface ControlGroupRendererProps {
timeRange?: TimeRange;
query?: Query;
dataLoading?: boolean;
className?: string;
}

export const ControlGroupRenderer = ({
Expand All @@ -51,7 +50,6 @@ export const ControlGroupRenderer = ({
query,
viewMode,
dataLoading,
className,
}: ControlGroupRendererProps) => {
const id = useMemo(() => uuidv4(), []);
const [regenerateId, setRegenerateId] = useState(uuidv4());
Expand Down Expand Up @@ -189,7 +187,6 @@ export const ControlGroupRenderer = ({
}}
hidePanelChrome
panelProps={{ hideLoader: true }}
className={className}
/>
);
};
3 changes: 0 additions & 3 deletions src/plugins/controls/public/react_controls/control_group/components/control_group.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ interface Props {
};
hasUnappliedSelections: boolean;
labelPosition: ControlStyle;
className?: string;
}

export function ControlGroup({
Expand All @@ -55,7 +54,6 @@ export function ControlGroup({
controlsManager,
labelPosition,
hasUnappliedSelections,
className,
}: Props) {
const [isInitialized, setIsInitialized] = useState(false);
const [autoApplySelections, controlsInOrder] = useBatchedPublishingSubjects(
Expand Down Expand Up @@ -156,7 +154,6 @@ export function ControlGroup({
controlsManager.setControlApi(id, controlApi);
}}
isControlGroupInitialized={isInitialized}
className={className}
/>
))}
</EuiFlexGroup>
Expand Down
5 changes: 2 additions & 3 deletions src/plugins/controls/public/react_controls/control_group/components/control_panel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ const DragHandle = ({
export const ControlPanel = <ApiType extends DefaultControlApi = DefaultControlApi>({
Component,
uuid,
className,
}: ControlPanelProps<ApiType>) => {
const [api, setApi] = useState<ApiType | null>(null);
const {
Expand Down Expand Up @@ -158,12 +157,11 @@ export const ControlPanel = <ApiType extends DefaultControlApi = DefaultControlA
data-test-subj="control-frame-title"
fullWidth
label={usingTwoLineLayout ? panelTitle || defaultPanelTitle || '...' : undefined}
display={className === 'observability-slo' ? undefined : 'rowCompressed'}
display="rowCompressed"
>
<EuiFormControlLayout
fullWidth
isLoading={Boolean(dataLoading)}
compressed={className === 'observability-slo' ? false : true}
className={classNames(
'controlFrame__formControlLayout',
{
Expand Down Expand Up @@ -196,6 +194,7 @@ export const ControlPanel = <ApiType extends DefaultControlApi = DefaultControlA
)}
</>
}
compressed
>
<>
{blockingError && (
Expand Down
3 changes: 1 addition & 2 deletions src/plugins/controls/public/react_controls/control_group/get_control_group_factory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ export const getControlGroupEmbeddableFactory = (services: {
autoApplySelections,
ignoreParentSettings,
} = initialRuntimeState;
const isObservabilitySLO = await services.core.application.capabilities.apm;

const autoApplySelections$ = new BehaviorSubject<boolean>(autoApplySelections);
const defaultDataViewId = await services.dataViews.getDefaultId();
const lastSavedControlsState$ = new BehaviorSubject<ControlPanelsState>(
Expand Down Expand Up @@ -298,7 +298,6 @@ export const getControlGroupEmbeddableFactory = (services: {
controlsManager={controlsManager}
hasUnappliedSelections={hasUnappliedSelections}
labelPosition={labelPosition}
className={isObservabilitySLO ? 'observability-slo' : undefined}
/>
);
},
Expand Down
8 changes: 1 addition & 7 deletions ..._controls/controls/data_controls/options_list_control/components/options_list_control.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,8 @@ import './options_list.scss';

export const OptionsListControl = ({
controlPanelClassName,
className,
}: {
controlPanelClassName: string;
className?: string;
}) => {
const popoverId = useMemo(() => htmlIdGenerator()(), []);
const { api, stateManager, displaySettings } = useOptionsListContext();
Expand Down Expand Up @@ -176,11 +174,7 @@ export const OptionsListControl = ({
);

return (
<EuiFilterGroup
fullWidth
className={controlPanelClassName}
compressed={className === 'observability-slo' ? false : true}
>
<EuiFilterGroup fullWidth compressed className={controlPanelClassName}>
<EuiInputPopover
id={popoverId}
ownFocus
Expand Down
8 changes: 2 additions & 6 deletions ...controls/controls/data_controls/options_list_control/get_options_list_control_factory.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,7 @@ export const getOptionsListControlFactory = (
hideExists$,
hideSort$
);

return (
<OptionsListControlContext.Provider
value={{
Expand All @@ -399,12 +400,7 @@ export const getOptionsListControlFactory = (
displaySettings: { placeholder, hideActionBar, hideExclude, hideExists, hideSort },
}}
>
<OptionsListControl
controlPanelClassName={controlPanelClassName}
className={
services.core.application.capabilities.apm ? 'observability-slo' : undefined
}
/>
<OptionsListControl controlPanelClassName={controlPanelClassName} />
</OptionsListControlContext.Provider>
);
},
Expand Down
4 changes: 1 addition & 3 deletions ...ic/react_controls/controls/data_controls/range_slider/components/range_slider_control.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ interface Props {
value: RangeValue | undefined;
uuid: string;
controlPanelClassName?: string;
className?: string;
}

export const RangeSliderControl: FC<Props> = ({
Expand All @@ -47,7 +46,6 @@ export const RangeSliderControl: FC<Props> = ({
value,
uuid,
controlPanelClassName,
className,
}: Props) => {
const rangeSliderRef = useRef<EuiDualRangeProps | null>(null);

Expand Down Expand Up @@ -196,7 +194,7 @@ export const RangeSliderControl: FC<Props> = ({
min={displayedMin}
max={displayedMax}
isLoading={isLoading}
compressed={className === 'observability-slo' ? false : true}
compressed
inputPopoverProps={{
className: controlPanelClassName,
panelMinWidth: MIN_POPOVER_WIDTH,
Expand Down
3 changes: 1 addition & 2 deletions ...blic/react_controls/controls/timeslider_control/components/time_slider_anchored_range.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ interface Props {
ticks: EuiRangeTick[];
timeRangeMin: number;
timeRangeMax: number;
className?: string;
}

export function TimeSliderAnchoredRange(props: Props) {
Expand All @@ -41,7 +40,7 @@ export function TimeSliderAnchoredRange(props: Props) {
max={props.timeRangeMax}
step={props.stepSize}
ticks={props.ticks}
compressed={props.className !== 'observability-slo' ? false : true}
compressed
/>
);
}
3 changes: 1 addition & 2 deletions ...eact_controls/controls/timeslider_control/components/time_slider_sliding_window_range.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ interface Props {
ticks: EuiRangeTick[];
timeRangeMin: number;
timeRangeMax: number;
className?: string;
}

export function TimeSliderSlidingWindowRange(props: Props) {
Expand All @@ -37,7 +36,7 @@ export function TimeSliderSlidingWindowRange(props: Props) {
step={props.stepSize}
ticks={props.ticks}
isDraggable
compressed={props.className !== 'observability-slo' ? true : false}
compressed
/>
);
}
1 change: 0 additions & 1 deletion src/plugins/controls/public/react_controls/controls/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,5 +96,4 @@ export interface ControlPanelProps<
> {
uuid: string;
Component: PanelCompatibleComponent<ApiType, PropsType>;
className?: string;
}
1 change: 0 additions & 1 deletion src/plugins/embeddable/public/react_embeddable_system/react_embeddable_renderer.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ export const ReactEmbeddableRenderer = <
* as it changes. This is **not** expected to change over the lifetime of the component.
*/
onAnyStateChange?: (state: SerializedPanelState<SerializedState>) => void;
className?: string;
}) => {
const cleanupFunction = useRef<(() => void) | null>(null);
const firstLoadCompleteTime = useRef<number | null>(null);
Expand Down
1 change: 0 additions & 1 deletion ...c/components/slo/burn_rate/alert_details/components/custom_panels/apm/embeddable_root.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ export function APMEmbeddableRoot({
type={embeddableId}
getParentApi={() => ({ getSerializedStateForChild: () => ({ rawState: input }) })}
hidePanelChrome={true}
className="observability-slo"
/>
);
}
Expand Down
48 changes: 41 additions & 7 deletions ...plugins/observability_solution/slo/public/pages/slos/components/common/quick_filters.scss
View file
Open in desktop
Copy link
Contributor

@Heenawter Heenawter Sep 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@shahzad31 Can you confirm that it is just the SLO page that should have non-compressed styling?

From my understanding, only the SLO page's controls should be impacted - but right now, the Alerts page also uses this QuickFilters component, so the styles have been expanded here, too (and I believe they explicitly prefer the compact styling).

@rshen If possible, I think it would be better to only target the SLO page and not the whole QuickFilters component since this is used a lot more places than just SLO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry - I think this is the SLO page, totally can be misunderstanding though 😶‍🌫️ ? x-pack/plugins/observability_solution/slo/public/pages/slos/components/common/quick_filters.scss

Copy link
Contributor

Choose a reason for hiding this comment

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

i still honestly don't understand why these custom styles are kept in SLO plugin, they should be moved where the control components are.

Copy link
Contributor

@nreese nreese Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@rshen91 One solution to remove custom styles from SLO would be to add a compressed prop to ControlGroupRenderer. Then, ControlGroupRenderer could pass compressed to control group embeddable by adding a compressed boolean in the parent returned from ReactEmbeddableRenderer.getParentApi. Finally, the control group embeddable could use compressed styles if the parentApi contains compressed key where typeof parentApi.compressed === boolean && parentApi.compressed

Copy link
Contributor

@Heenawter Heenawter Sep 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

@rshen91 @shahzad31 fyi What @nreese is describing is a potential solution for #189939 that he came up with and we discussed offline :) I think it could work really well for a case like this - we can add a compressed prop to ControlGroupRenderer without having to add "serializable state" to the control group, which is what we were originally trying to avoid with these custom styles.

I think this is the best plan moving forward - sorry for the back and forth 🙇

Copy link
Contributor

Choose a reason for hiding this comment

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

Here is an example for MapRenderer that migrates props from serialized state to parent Api.

Original file line number Diff line number Diff line change
@@ -1,26 +1,60 @@
.controlsWrapper {
Heenawter marked this conversation as resolved.
Show resolved Hide resolved
min-height: $euiSize * 4;
}

.controlsFrame__draggable {
height: $euiButtonHeight;
height: $euiButtonHeight !important;
}

.optionsList--filterBtn {
height: $euiButtonHeight;
.optionsList--filterBtn, .euiFilterButton {
height: $euiButtonHeight !important;
}

.euiFilterGroup {
height: $euiButtonHeight !important;
border-radius: $euiFormControlBorderRadius !important;
}

.controlPanel {
height: $euiButtonHeight;
height: $euiButtonHeight !important;

.euiDualRange, .euiRange {
height: $euiButtonHeight !important;
}

&--labelWrapper {
height: 100%;
height: 100% !important;
}

.controlPanel--label {
height: 100%;
height: 100% !important;
}
}

.dshDashboardViewport-controls {
Heenawter marked this conversation as resolved.
Show resolved Hide resolved
padding-bottom: $euiSizeXS;
}
}

.euiFlexGroup {
height: $euiButtonHeight;
}

.euiFormControlLayout, .euiFormControlLayout--group, .controlFrame__formControlLayout, .optionsListControl {
height: $euiButtonHeight !important;
}

.optionsListControl {
height: $euiButtonHeight !important;
}

.--group {
height: $euiButtonHeight !important;
}

.controlFrame__formControlLayout {
height: $euiButtonHeight !important;
}

&.-compresed {
height: $euiButtonHeight !important;
}
1 change: 0 additions & 1 deletion .../plugins/observability_solution/slo/public/pages/slos/components/common/quick_filters.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ export function QuickFilters({
return (
<Container>
<ControlGroupRenderer
className="observability-slo"
onApiAvailable={setControlGroupAPI}
getCreationOptions={async (initialState, builder) => {
builder.addOptionsListControl(
Expand Down