-
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
[ML][UX]: Consistent Layout and UI Enhancements for ML Pages #203813
Changes from 17 commits
6a0d029
28d27b0
924c396
a3b2030
02aa1f3
3ae73e8
0e0f1de
9960e5c
a0bb316
18fe658
9142f10
a0bea28
9aa70f6
63c3193
4db66c5
24eaf3d
e9108d6
4c193b7
7f408c6
cd56f03
dfd2494
847d00e
d73ab5d
7f47fad
e679f8e
c9bf590
1368a37
f54f968
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 |
---|---|---|
|
@@ -20,7 +20,6 @@ import { | |
EuiPanel, | ||
EuiProgress, | ||
EuiSpacer, | ||
EuiTitle, | ||
} from '@elastic/eui'; | ||
|
||
import { type Filter, FilterStateStore, type Query, buildEsQuery } from '@kbn/es-query'; | ||
|
@@ -37,7 +36,6 @@ import { useStorage } from '@kbn/ml-local-storage'; | |
import type { SavedSearch } from '@kbn/saved-search-plugin/public'; | ||
import { SEARCH_QUERY_LANGUAGE, type SearchQueryLanguage } from '@kbn/ml-query-utils'; | ||
import { kbnTypeToSupportedType } from '../../../common/util/field_types_utils'; | ||
import { useCurrentEuiTheme } from '../../../common/hooks/use_current_eui_theme'; | ||
import { | ||
DV_FROZEN_TIER_PREFERENCE, | ||
DV_RANDOM_SAMPLER_PREFERENCE, | ||
|
@@ -108,8 +106,6 @@ export interface IndexDataVisualizerViewProps { | |
} | ||
|
||
export const IndexDataVisualizerView: FC<IndexDataVisualizerViewProps> = (dataVisualizerProps) => { | ||
const euiTheme = useCurrentEuiTheme(); | ||
|
||
const [savedRandomSamplerPreference, saveRandomSamplerPreference] = useStorage< | ||
DVKey, | ||
DVStorageMapped<typeof DV_RANDOM_SAMPLER_PREFERENCE> | ||
|
@@ -515,49 +511,40 @@ export const IndexDataVisualizerView: FC<IndexDataVisualizerViewProps> = (dataVi | |
paddingSize="none" | ||
> | ||
<EuiPageTemplate.Section> | ||
<EuiPageTemplate.Header data-test-subj="dataVisualizerPageHeader" css={dvPageHeader}> | ||
<EuiFlexGroup | ||
data-test-subj="dataViewTitleHeader" | ||
direction="row" | ||
alignItems="center" | ||
css={{ padding: `${euiTheme.euiSizeS} 0`, marginRight: `${euiTheme.euiSize}` }} | ||
> | ||
<EuiTitle size={'s'}> | ||
<h2>{currentDataView.getName()}</h2> | ||
</EuiTitle> | ||
<DataVisualizerDataViewManagement currentDataView={currentDataView} /> | ||
</EuiFlexGroup> | ||
|
||
{isWithinLargeBreakpoint ? <EuiSpacer size="m" /> : null} | ||
<EuiFlexGroup | ||
alignItems="center" | ||
justifyContent="flexEnd" | ||
gutterSize="s" | ||
data-test-subj="dataVisualizerTimeRangeSelectorSection" | ||
> | ||
{hasValidTimeField ? ( | ||
<EuiFlexItem grow={false}> | ||
<FullTimeRangeSelector | ||
frozenDataPreference={frozenDataPreference} | ||
setFrozenDataPreference={setFrozenDataPreference} | ||
dataView={currentDataView} | ||
query={undefined} | ||
disabled={false} | ||
timefilter={timefilter} | ||
/> | ||
</EuiFlexItem> | ||
) : null} | ||
<EuiFlexItem grow={false}> | ||
<DatePickerWrapper | ||
isAutoRefreshOnly={!hasValidTimeField} | ||
showRefresh={!hasValidTimeField} | ||
width="full" | ||
needsUpdate={queryNeedsUpdate} | ||
onRefresh={handleRefresh} | ||
<EuiPageTemplate.Header | ||
data-test-subj="dataVisualizerPageHeader" | ||
css={dvPageHeader} | ||
pageTitle={ | ||
<> | ||
{currentDataView.getName()} | ||
{/* TODO: This management section shouldn't live inside the header */} | ||
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. Are you planning on doing this in this PR? 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 think we can tackle it, but it would be nice to find a different place for this menu to reside. 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 agree that we need to find a different place for this menu. We can do this in a separate issue. |
||
<DataVisualizerDataViewManagement currentDataView={currentDataView} /> | ||
</> | ||
} | ||
rightSideGroupProps={{ | ||
gutterSize: 's', | ||
'data-test-subj': 'dataComparisonTimeRangeSelectorSection', | ||
}} | ||
rightSideItems={[ | ||
<DatePickerWrapper | ||
isAutoRefreshOnly={!hasValidTimeField} | ||
showRefresh={!hasValidTimeField} | ||
width="full" | ||
needsUpdate={queryNeedsUpdate} | ||
onRefresh={handleRefresh} | ||
/>, | ||
hasValidTimeField && ( | ||
<FullTimeRangeSelector | ||
frozenDataPreference={frozenDataPreference} | ||
setFrozenDataPreference={setFrozenDataPreference} | ||
dataView={currentDataView} | ||
query={undefined} | ||
disabled={false} | ||
timefilter={timefilter} | ||
/> | ||
</EuiFlexItem> | ||
</EuiFlexGroup> | ||
</EuiPageTemplate.Header> | ||
), | ||
]} | ||
/> | ||
<EuiSpacer size="m" /> | ||
|
||
<EuiFlexGroup gutterSize="m" direction={isWithinLargeBreakpoint ? 'column' : 'row'}> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0; you may not use this file except in compliance with the Elastic License | ||
* 2.0. | ||
*/ | ||
|
||
/** | ||
* Titles for the cases toast messages | ||
*/ | ||
export const TITLES = { | ||
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. Suggest to make this a bit more specific like 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. Agree, done in: #7f47fad |
||
CHANGE_POINT_DETECTION: 'Change point detection', | ||
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. Should these be internationalized? 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.
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. Done in: #7f408c6 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'd forgotten that it's possible to multi-select charts to add to a case - can a plural condition be added for 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. Added plural for |
||
LOG_RATE_ANALYSIS: 'Log rate analysis', | ||
PATTERN_ANALYSIS: 'Log pattern analysis', | ||
}; |
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.
Looks like you accidentaly removed the comment for the next prop and not
flexGroup
.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.
Good catch, done in: #d73ab5d