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.

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

[SharedUX] EUI visual refresh for SharedUX #202780

Merged
merged 27 commits into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e58f61a
[SharedUX] EUI visual refresh for SharedUX
tsullivan Nov 27, 2024
ea3654b
update kibana package type for button_toolbar
tsullivan Dec 4, 2024
57be122
[CI] Auto-commit changed files from 'node scripts/notice'
kibanamachine Dec 4, 2024
e33c2ca
[CI] Auto-commit changed files from 'node scripts/yarn_deduplicate'
kibanamachine Dec 4, 2024
abc049b
[CI] Auto-commit changed files from 'node scripts/lint_ts_projects --…
kibanamachine Dec 4, 2024
c3704f1
update snapshot
tsullivan Dec 4, 2024
74bf238
WIP fix jest test
tsullivan Dec 4, 2024
7a70b81
update package type for File Upload
tsullivan Dec 4, 2024
1b69f1c
updates per designer feedback
tsullivan Dec 5, 2024
b4798c8
strip out changes to src/plugins/home/public/application/components
tsullivan Dec 5, 2024
9a7a624
fix package type lint
tsullivan Dec 5, 2024
cb86d84
update setup guide button colors
tsullivan Dec 5, 2024
f2338db
update color for bottom bar button in advanced settings
tsullivan Dec 5, 2024
d9fdff3
update guide button to "accent"
tsullivan Dec 6, 2024
21dba67
add fill to file upload buttons
tsullivan Dec 6, 2024
221f8b4
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 7, 2024
b793f95
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 9, 2024
4d9f373
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 9, 2024
7ef630b
import euiFormMaxWidth from EUI form styles
tsullivan Dec 9, 2024
06778d3
correction to import path
tsullivan Dec 9, 2024
ddf07f5
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 10, 2024
46ea1b4
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 10, 2024
73d9d65
revert import to test build metrics
tsullivan Dec 10, 2024
fb95f53
add FIXME comment
tsullivan Dec 10, 2024
06858e7
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 11, 2024
d5ee36b
Use global typings for emotion
tsullivan Dec 11, 2024
ec429d3
Merge branch 'main' into sharedux/eui-visual-refresh-audit
tsullivan Dec 12, 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
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export const BottomBar = ({
<EuiButton
css={cssFormButton}
disabled={hasInvalidChanges}
color="success"
color="primary"
Copy link
Member Author

@tsullivan tsullivan Dec 6, 2024

Choose a reason for hiding this comment

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

settings-bottom-bar.mov

fill
size="s"
iconType="check"
Expand Down
4 changes: 2 additions & 2 deletions packages/shared-ux/button_toolbar/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"type": "shared-common",
"type": "shared-browser",
"id": "@kbn/shared-ux-button-toolbar",
"owner": [
"@elastic/appex-sharedux"
],
"group": "platform",
"visibility": "shared"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ export const ToolbarButtonStyles = ({ euiTheme }: UseEuiTheme) => {
borderColor: euiTheme.border.color,
},
emptyButton: {
backgroundColor: euiTheme.colors.emptyShade,
backgroundColor: euiTheme.colors.backgroundBasePlain,
border: `${euiTheme.border.thin}`,
color: `${euiTheme.colors.text}`,
color: `${euiTheme.colors.textParagraph}`,
Copy link
Member Author

@tsullivan tsullivan Dec 4, 2024

Choose a reason for hiding this comment

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

Accessing this from Kibana:

toolbar-buttons.mov

},
buttonPositions: {
left: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@ export const LabelBadge = ({
className?: string;
}) => {
const { euiTheme } = useEuiTheme();

return (
<EuiBetaBadge
label={text}
size="s"
css={css`
margin-left: ${euiTheme.size.s};
color: ${euiTheme.colors.text};
color: ${euiTheme.colors.textParagraph};
Copy link
Member Author

@tsullivan tsullivan Dec 4, 2024

Choose a reason for hiding this comment

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

I'm not sure we are actually using this component anywhere currently, but it could be somewhere in the navigation panel for solutions:

navigation-panels.mov

vertical-align: middle;
margin-bottom: ${euiTheme.size.xxs};
`}
Expand Down
2 changes: 1 addition & 1 deletion packages/shared-ux/code_editor/impl/placeholder_widget.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export class PlaceholderWidget implements monaco.editor.IContentWidget {
const domNode = document.createElement('div');
domNode.innerText = this.placeholderText;
domNode.className = css`
color: ${this.euiTheme.colors.subduedText};
color: ${this.euiTheme.colors.textSubdued};
width: max-content;
pointer-events: none;
`;
Expand Down
4 changes: 2 additions & 2 deletions packages/shared-ux/file/file_picker/impl/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"type": "shared-common",
"type": "shared-browser",
"id": "@kbn/shared-ux-file-picker",
"owner": [
"@elastic/appex-sharedux"
],
"group": "platform",
"visibility": "shared"
}
}
4 changes: 2 additions & 2 deletions packages/shared-ux/file/file_upload/impl/kibana.jsonc
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
{
"type": "shared-common",
"type": "shared-browser",
"id": "@kbn/shared-ux-file-upload",
"owner": [
"@elastic/appex-sharedux"
],
"group": "platform",
"visibility": "shared"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,13 @@ export const CancelButton: FunctionComponent<Props> = ({ onClick, compressed })
/>
) : (
<EuiButton
color="danger"
fill={true}
key="cancelButton"
size="s"
data-test-subj="cancelButton"
disabled={disabled}
onClick={onClick}
color="danger"
>
{i18nTexts.cancel}
</EuiButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ export const UploadButton: FunctionComponent<Props> = ({ onClick }) => {
key="uploadButton"
isLoading={uploading}
color={done ? 'success' : 'primary'}
fill={true}
iconType={done ? 'checkInCircleFilled' : undefined}
disabled={Boolean(!files.length || error || done)}
onClick={onClick}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,30 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { css } from '@emotion/react';
import React from 'react';
import useObservable from 'react-use/lib/useObservable';

import {
EuiText,
EuiSpacer,
EuiFlexItem,
EuiFlexGroup,
EuiFilePicker,
EuiFlexGroup,
EuiFlexItem,
EuiSpacer,
EuiText,
useEuiTheme,
useGeneratedHtmlId,
mathWithUnits,
} from '@elastic/eui';
import type {
EuiFilePickerClass,
EuiFilePickerProps,
} from '@elastic/eui/src/components/form/file_picker/file_picker';
import { euiThemeVars } from '@kbn/ui-theme';

import { useBehaviorSubject } from '@kbn/shared-ux-file-util';
import { css } from '@emotion/react';
import useObservable from 'react-use/lib/useObservable';
import { i18nTexts } from './i18n_texts';
import { ControlButton, ClearButton } from './components';

import { ClearButton, ControlButton } from './components';
import { useUploadState } from './context';
import { i18nTexts } from './i18n_texts';

export interface Props {
meta?: unknown;
Expand All @@ -41,8 +44,6 @@ export interface Props {
className?: string;
}

const { euiFormMaxWidth, euiButtonHeightSmall } = euiThemeVars;

const styles = {
horizontalContainer: css`
display: flex;
Expand Down Expand Up @@ -79,12 +80,15 @@ export const FileUpload = React.forwardRef<EuiFilePickerClass, Props>(
const id = useGeneratedHtmlId({ prefix: 'filesFileUpload' });
const errorId = `${id}_error`;

// FIXME: add a token for this on euiTheme.components. https://github.com/elastic/eui/issues/8217
const formMaxWidth = mathWithUnits(euiTheme.size.base, (x) => x * 25);
Copy link
Member Author

Choose a reason for hiding this comment

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

This copies the euiFormMaxWidth function in EUI: https://github.com/elastic/eui/blob/7ec3fafaac4d6a31d5d324a118c5cb98b7395922/packages/eui/src/components/form/form.styles.ts#L28

@elastic/eui-team is there a way we can access this through the EUI context?

Copy link
Contributor

Choose a reason for hiding this comment

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

ℹ️ We're introducing a new token for it euiTheme.components.forms.maxWidth (PR).


return (
<div
data-test-subj="filesFileUpload"
css={[
css`
max-width: ${fullWidth ? '100%' : euiFormMaxWidth};
max-width: ${fullWidth ? '100%' : formMaxWidth};
`,
fullWidth ? styles.fullWidth : undefined,
compressed ? styles.horizontalContainer : undefined,
Expand Down Expand Up @@ -143,7 +147,7 @@ export const FileUpload = React.forwardRef<EuiFilePickerClass, Props>(
css={css`
display: flex;
align-items: center;
min-height: ${euiButtonHeightSmall};
min-height: ${euiTheme.size.xl};
Copy link
Member Author

Choose a reason for hiding this comment

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

I tested this component through the developer examples. To do this, you have to run Kibana with the --run-examples flag

file-example.mov

`}
size="s"
color="danger"
Expand Down
1 change: 0 additions & 1 deletion packages/shared-ux/file/file_upload/impl/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
],
"kbn_references": [
"@kbn/i18n",
"@kbn/ui-theme",
"@kbn/shared-ux-file-context",
"@kbn/shared-ux-file-util",
"@kbn/shared-ux-file-types",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export const GuideButton = ({
<EuiButton
isLoading={isLoading}
onClick={toggleGuidePanel}
color="success"
color="accent"
Copy link
Contributor

Choose a reason for hiding this comment

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

❓ Is this expected? Or should it rather be accentSecondary? @andreadelrio
I thought accent buttons shouldn't really be used? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

My understanding is that we don't know for certain if we will make accentSecondary an available button given the numerous concerns raised by designers about having two greens (particularly in buttons). Therefore I think it's safer to use the accent button which has existed in the system for a long time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, yeah I think I was behind on latest button color decisions 😅 I also just saw this (message).

fill
size="s"
data-test-subj="guideButton"
Expand Down Expand Up @@ -97,7 +97,7 @@ export const GuideButton = ({
return (
<EuiButton
onClick={navigateToLandingPage}
color="success"
color="accent"
Copy link
Member Author

Choose a reason for hiding this comment

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

setup-guide-button.mov

fill
size="s"
data-test-subj="guideButtonRedirect"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,21 @@ export const getGuidePanelStepStyles = (euiTheme: EuiThemeComputed, stepStatus:
height: 24px;
border-radius: 50%;
border: 2px solid
${stepStatus === 'inactive' ? euiTheme.colors.lightShade : euiTheme.colors.success};
${stepStatus === 'inactive' ? euiTheme.colors.borderBasePlain : euiTheme.colors.success};
font-weight: ${euiTheme.font.weight.medium};
text-align: center;
line-height: 1.4;
color: ${stepStatus === 'inactive' ? euiTheme.colors.subduedText : euiTheme.colors.text};
color: ${stepStatus === 'inactive'
? euiTheme.colors.textSubdued
: euiTheme.colors.textParagraph};
`,
stepTitle: css`
font-weight: ${euiTheme.font.weight.semiBold};
color: ${stepStatus === 'inactive' ? euiTheme.colors.subduedText : euiTheme.colors.text};
color: ${stepStatus === 'inactive'
? euiTheme.colors.textSubdued
: euiTheme.colors.textParagraph};
.euiAccordion-isOpen & {
color: ${euiTheme.colors.title};
color: ${euiTheme.colors.textHeading};
Copy link
Member Author

Choose a reason for hiding this comment

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

How to access this UI:

guide-panel-step.mov

}
`,
description: css`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ export const QuitGuideModal = ({
onClick={deactivateGuide}
isLoading={isLoading}
fill
color="warning"
color="primary"
Copy link
Member Author

Choose a reason for hiding this comment

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

This change request came from @andreadelrio

>
{i18n.translate('guidedOnboarding.quitGuideModal.quitButtonLabel', {
defaultMessage: 'Quit guide',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import * as StatusCheckStates from './status_check_states';

import { injectI18n, FormattedMessage } from '@kbn/i18n-react';
import { euiThemeVars } from '@kbn/ui-theme';
import { euiThemeVars } from '@kbn/ui-theme'; // FIXME: remove this, and access style variables from EUI context
Copy link
Contributor

Choose a reason for hiding this comment

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

We might be able to be rid of this, if we adopt the css style function callback on L281

Copy link
Member Author

Choose a reason for hiding this comment

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

@eokoneyo that works fine as a solution to remove this import, but unfortunately that has heavy impact to the Jest unit test for the src/plugins/home/public/application/components/tutorial/tutorial.test.js file. The test would need to mount the component wrapped in <EuiProvider>, which changes the structure of the component, and the tests themselves are using Enzyme and are heavily dependent on the structure of the component.

This entire plugin is on the roadmap to be rewritten into TypeScript in the next quarter. Maybe just adding this FIXME comment is the right thing to do for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

@tsullivan right... it's probably best to resolve this much later then


class InstructionSetUi extends React.Component {
constructor(props) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
}
&.securitySolution {
.euiCard__image {
background-color: $euiColorSuccess;
background-color: $euiColorAccentSecondary;
Copy link
Member Author

Choose a reason for hiding this comment

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

analytics-overview.mov

}
}
}
Expand Down
48 changes: 24 additions & 24 deletions src/plugins/navigation/public/side_navigation/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,34 +8,34 @@
*/

import { css } from '@emotion/react';
import { euiThemeVars } from '@kbn/ui-theme';
import React, { Suspense, type FC } from 'react';
import { EuiSkeletonRectangle } from '@elastic/eui';
import { EuiSkeletonRectangle, useEuiTheme } from '@elastic/eui';
import { i18n } from '@kbn/i18n';

import type { Props as NavigationProps } from './side_navigation';

const SideNavComponentLazy = React.lazy(() => import('./side_navigation'));

export const SideNavComponent: FC<NavigationProps> = (props) => (
<Suspense
fallback={
<EuiSkeletonRectangle
css={css`
margin: ${euiThemeVars.euiSize};
`}
width={16}
height={16}
borderRadius="s"
contentAriaLabel={i18n.translate(
'navigation.sideNavigation.loadingSolutionNavigationLabel',
{
defaultMessage: 'Loading solution navigation',
}
)}
/>
}
>
<SideNavComponentLazy {...props} />
</Suspense>
);
export const SideNavComponent: FC<NavigationProps> = (props) => {
const { euiTheme } = useEuiTheme();
return (
<Suspense
fallback={
<EuiSkeletonRectangle
css={css`
margin: ${euiTheme.size.base};
`}
width={16}
height={16}
borderRadius="s"
contentAriaLabel={i18n.translate(
'navigation.sideNavigation.loadingSolutionNavigationLabel',
{ defaultMessage: 'Loading solution navigation' }
)}
/>
}
>
<SideNavComponentLazy {...props} />
</Suspense>
);
Copy link
Member Author

Choose a reason for hiding this comment

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

This just affects the loading state of the solution side nav. You will need to throttle the browser's network or play back a video frame-by-frame to see it.

side-nav-loading-state.mov

};
1 change: 0 additions & 1 deletion src/plugins/navigation/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
"@kbn/config-schema",
"@kbn/i18n",
"@kbn/std",
"@kbn/ui-theme",
],
"exclude": [
"target/**/*",
Expand Down
16 changes: 16 additions & 0 deletions src/plugins/saved_objects/emotion.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/*
* 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", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import '@emotion/react';
import type { UseEuiTheme } from '@elastic/eui';

declare module '@emotion/react' {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
export interface Theme extends UseEuiTheme {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import { FormattedMessage } from '@kbn/i18n-react';
import React from 'react';
import { EuiText } from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { euiThemeVars } from '@kbn/ui-theme';

export interface OnSaveProps {
newTitle: string;
Expand Down Expand Up @@ -405,7 +404,10 @@ export class SavedObjectSaveModal extends React.Component<Props, SaveModalState>
/>
</EuiFlexItem>
{this.props.mustCopyOnSaveMessage && (
<EuiFlexItem css={{ marginLeft: `-${euiThemeVars.euiSize}` }} grow={false}>
<EuiFlexItem
css={({ euiTheme }) => ({ marginLeft: `-${euiTheme.size.base}` })}
grow={false}
>
<EuiIconTip type="iInCircle" content={this.props.mustCopyOnSaveMessage} />
Copy link
Member Author

Choose a reason for hiding this comment

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

This message was added by #175062. It looks like this code might no longer be reachable, as there seems to be no longer an "Edit" button when viewing managed content (tested with a managed dashboard). There is only a "Duplicate" button.

</EuiFlexItem>
)}
Expand Down
15 changes: 10 additions & 5 deletions src/plugins/saved_objects/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,20 +1,25 @@
{
"extends": "../../../tsconfig.base.json",
"compilerOptions": {
"outDir": "target/types",
"outDir": "target/types"
},
"include": ["common/**/*", "public/**/*", "server/**/*"],
"include": [
"common/**/*",
"public/**/*",
"server/**/*",
// Emotion theme typing
"./emotion.d.ts"
],
"kbn_references": [
"@kbn/core",
"@kbn/data-plugin",
"@kbn/i18n",
"@kbn/data-views-plugin",
"@kbn/i18n-react",
"@kbn/ui-theme",
"@kbn/react-kibana-mount",
"@kbn/test-jest-helpers",
"@kbn/test-jest-helpers"
],
"exclude": [
"target/**/*",
"target/**/*"
]
}
Loading