-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Observability] fix slo observability compressed styles to be not compressed #193081
Changes from 12 commits
2fb8a5a
67ea6a2
cf01d44
c9d3ffe
0b6fbc0
8c45a2a
5354eb0
e590230
caa0bf2
7f0d2f3
2831847
a722e87
913f0ff
bb6a928
451d33c
c9a2d59
4cf021a
cf410d8
9c60c87
83da3f5
02f6a67
9e32829
c986f23
ad6ee2c
899b248
adc1846
bb77b0d
b21f593
00c0670
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -199,6 +199,8 @@ export const getControlGroupEmbeddableFactory = () => { | |
references, | ||
}; | ||
}, | ||
compressed: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't expose via control group api. Anywhere its needed, grab property from parent. |
||
parentApi && typeof parentApi.compressed === 'boolean' ? parentApi.compressed : true, | ||
dataViews, | ||
labelPosition: labelPosition$, | ||
saveNotification$: apiHasSaveNotification(parentApi) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 @rshen If possible, I think it would be better to only target the SLO page and not the whole There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 😶🌫️ ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 I think this is the best plan moving forward - sorry for the back and forth 🙇 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
---|---|---|
@@ -0,0 +1,13 @@ | ||
|
||
.controlPanel--labelWrapper > label { | ||
height: $euiFormControlHeight !important; | ||
line-height: 3 !important; | ||
} | ||
|
||
.optionsListControl { | ||
height: $euiFormControlHeight !important; | ||
} | ||
|
||
.optionsList--selectionText { | ||
padding-top: 5px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is not needed.
compressed
should not be something exposed from the ControlGroupApi. Its only used internally by the control group. Its value comes from the parent. Consumers do not need to know about this implementation detail.