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

[EuiBeacon] Add color prop #6420

Merged
merged 5 commits into from
Nov 23, 2022
Merged

[EuiBeacon] Add color prop #6420

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c6f5542
[EuiBeacon] Add color prop
dawitamene Nov 21, 2022
ac3f76b
Add CL entry
dawitamene Nov 21, 2022
002654c
Omit color field from HTMLAttributes
dawitamene Nov 21, 2022
37b613f
Remove color parameter from Emotion style function and return styles …
dawitamene Nov 22, 2022
c52d847
Merge remote-tracking branch 'upstream/main' into beacon/color-prop
cee-chen Nov 22, 2022
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
51 changes: 48 additions & 3 deletions src/components/beacon/__snapshots__/beacon.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,64 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`EuiBeacon accepts size 1`] = `
exports[`EuiBeacon is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:14px;width:14px"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon is rendered 1`] = `
exports[`EuiBeacon props color accent is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon props color danger is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon props color primary is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon props color success is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon props color warning is rendered 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:12px;width:12px"
/>
`;

exports[`EuiBeacon props size accepts size 1`] = `
<div
aria-label="aria-label"
class="euiBeacon testClass1 testClass2 emotion-euiBeacon"
data-test-subj="test subject string"
style="height:14px;width:14px"
/>
`;
14 changes: 8 additions & 6 deletions src/components/beacon/beacon.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@
*/

import { keyframes, css } from '@emotion/react';
import { euiPaletteColorBlind } from '../../services';
import { UseEuiTheme } from '../../services';
import {
euiCanAnimate,
logicalCSS,
logicalSizeCSS,
} from '../../global_styling';

const visColors = euiPaletteColorBlind();
import { EuiBeaconColor } from './beacon';

const euiBeaconPulseLarge = keyframes`
0% {
Expand Down Expand Up @@ -48,11 +47,14 @@ const euiBeaconPulseSmall = keyframes`
}
`;

export const euiBeaconStyles = () => ({
export const euiBeaconStyles = (
{ euiTheme }: UseEuiTheme,
color: EuiBeaconColor
) => ({
cee-chen marked this conversation as resolved.
Show resolved Hide resolved
// Base
euiBeacon: css`
position: relative;
background-color: ${visColors[0]};
background-color: ${euiTheme.colors[color]};
border-radius: 50%;

&:before,
Expand All @@ -64,7 +66,7 @@ export const euiBeaconStyles = () => ({
${logicalCSS('top', 0)}
background-color: transparent;
border-radius: 50%;
box-shadow: 0 0 1px 1px ${visColors[0]};
box-shadow: 0 0 1px 1px ${euiTheme.colors[color]};
}

// Without the animation, we only display one ring around the circle
Expand Down
23 changes: 19 additions & 4 deletions src/components/beacon/beacon.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { render } from 'enzyme';
import { requiredProps } from '../../test/required_props';
import { shouldRenderCustomStyles } from '../../test/internal';

import { EuiBeacon } from './beacon';
import { EuiBeacon, COLORS } from './beacon';

describe('EuiBeacon', () => {
shouldRenderCustomStyles(<EuiBeacon />);
Expand All @@ -22,9 +22,24 @@ describe('EuiBeacon', () => {
expect(component).toMatchSnapshot();
});

test('accepts size', () => {
const component = render(<EuiBeacon size={14} {...requiredProps} />);
describe('props', () => {
describe('color', () => {
COLORS.forEach((color) => {
it(`${color} is rendered`, () => {
const component = render(
<EuiBeacon color={color} {...requiredProps} />
);

expect(component).toMatchSnapshot();
expect(component).toMatchSnapshot();
});
});
});
describe('size', () => {
it('accepts size', () => {
const component = render(<EuiBeacon size={14} {...requiredProps} />);

expect(component).toMatchSnapshot();
});
});
Comment on lines +37 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Very minor / optional code organization - I'd add a newline between blocks and leave this as its own test vs giving it a describe block

Suggested change
describe('size', () => {
it('accepts size', () => {
const component = render(<EuiBeacon size={14} {...requiredProps} />);
expect(component).toMatchSnapshot();
});
});
test('size', () => {
const component = render(<EuiBeacon size={14} {...requiredProps} />);
expect(component).toMatchSnapshot();
});

Copy link
Contributor Author

@dawitamene dawitamene Nov 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@constancecchen I agree. I tried to commit, but build process is failing with this error.

src/components/modal/modal.tsx:84:60 - error TS2769: No overload matches this call.
  Overload 1 of 2, '(props: EuiFocusTrapProps | Readonly<EuiFocusTrapProps>): EuiFocusTrap', gave the following error.
    Type '{ children: Element; initialFocus: string | HTMLElement | (() => HTMLElement) | undefined; scrollLock: true; preventScrollOnFocus: true; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<EuiFocusTrap> & Pick<Readonly<EuiFocusTrapProps> & Readonly<...>, "className" | ... 17 more ... | "closeOnMouseup"> & Partial<...> & Partial<...>'.
      Property 'preventScrollOnFocus' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<EuiFocusTrap> & Pick<Readonly<EuiFocusTrapProps> & Readonly<...>, "className" | ... 17 more ... | "closeOnMouseup"> & Partial<...> & Partial<...>'.
  Overload 2 of 2, '(props: EuiFocusTrapProps, context: any): EuiFocusTrap', gave the following error.
    Type '{ children: Element; initialFocus: string | HTMLElement | (() => HTMLElement) | undefined; scrollLock: true; preventScrollOnFocus: true; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes<EuiFocusTrap> & Pick<Readonly<EuiFocusTrapProps> & Readonly<...>, "className" | ... 17 more ... | "closeOnMouseup"> & Partial<...> & Partial<...>'.
      Property 'preventScrollOnFocus' does not exist on type 'IntrinsicAttributes & IntrinsicClassAttributes<EuiFocusTrap> & Pick<Readonly<EuiFocusTrapProps> & Readonly<...>, "className" | ... 17 more ... | "closeOnMouseup"> & Partial<...> & Partial<...>'.

84       <EuiFocusTrap initialFocus={initialFocus} scrollLock preventScrollOnFocus>
                                                              ~~~~~~~~~~~~~~~~~~~~



Found 1 error.

image

I checked EuiFocusTrap's props and there's no preventScrollOnFocus prop.

Copy link
Member

Choose a reason for hiding this comment

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

That's a super bizarre error since it's not related to EuiBeacon 🤔 - I recently merged in latest main into this branch in order to kickstart CI, so I'm guessing what happened is you may need to re-run yarn to fix type definitions.

No worries at all though as this was a super optional comment! Thank you again for your great work!

});
});
24 changes: 22 additions & 2 deletions src/components/beacon/beacon.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,23 +11,43 @@ import { CommonProps } from '../common';
import classNames from 'classnames';

import { euiBeaconStyles } from './beacon.styles';
import { useEuiTheme } from '../../services';

export type EuiBeaconProps = Omit<HTMLAttributes<HTMLDivElement>, 'children'> &
export const COLORS = [
'primary',
'success',
'accent',
'danger',
'warning',
] as const;

export type EuiBeaconColor = typeof COLORS[number];

export type EuiBeaconProps = Omit<
HTMLAttributes<HTMLDivElement>,
'children' | 'color'
> &
CommonProps & {
/**
* Height and width of the center circle. Value is passed directly to the `style` attribute
*/
size?: number | string;
/**
* Any of the named color palette options.
*/
color?: EuiBeaconColor;
};

export const EuiBeacon: FunctionComponent<EuiBeaconProps> = ({
className,
size = 12,
color = 'success',
style,
...rest
}) => {
const euiTheme = useEuiTheme();
const classes = classNames('euiBeacon', className);
const styles = euiBeaconStyles();
const styles = euiBeaconStyles(euiTheme, color);
const cssStyles = [styles.euiBeacon];
cee-chen marked this conversation as resolved.
Show resolved Hide resolved

const beaconStyle = {
Expand Down
1 change: 1 addition & 0 deletions upcoming_changelogs/6420.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Added `color` prop to `EuiBeacon`