[EuiBeacon] Add color prop by dawitamene · Pull Request #6420 · elastic/eui

dawitamene · 2022-11-21T13:34:14Z

Summary

This PR adds an optional color prop to EuiBeacon component and closes #6315.

General checklist

Checked in both light and dark modes
Checked in mobile
Checked in Chrome, Safari, Edge, and Firefox
Props have proper autodocs and playground toggles
Added documentation
~~- [ ] Checked Code Sandbox works for any docs examples~~
Added or updated jest and cypress tests
~~- [ ] Checked for accessibility including keyboard-only and screenreader modes~~
Updated the Figma library counterpart
A changelog entry exists and is marked appropriately

kibanamachine · 2022-11-21T13:34:16Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

cla-checker-service · 2022-11-21T13:34:18Z

💚 CLA has been signed

dawitamene · 2022-11-21T14:14:05Z

@miukimiu

I'm not sure if this is a bug or if I'm missing something here, but the color prop doesn't show up in the documentation site. Any other prop name works fine except for color. Here's a screenshot with color1 as prop for example.

src/components/beacon/beacon.tsx

src/components/beacon/beacon.styles.ts

src/components/beacon/beacon.tsx

…with individual color keys instead

cee-chen · 2022-11-22T17:55:31Z

jenkins test this

cee-chen

This looks great to me from a code POV. @miukimiu, do you think this new prop needs any more documentation than it currently has (just props table) or is it good to go?

cee-chen · 2022-11-22T18:10:24Z

src/components/beacon/beacon.test.tsx

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


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();

});

@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.

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

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!

kibanamachine · 2022-11-22T18:23:44Z

Preview documentation changes for this PR: https://eui.elastic.co/pr_6420/

elizabetdev · 2022-11-22T19:55:44Z

@miukimiu, do you think this new prop needs any more documentation than it currently has (just props table) or is it good to go?

I think for now it's ok to just have the props table.

cee-chen · 2022-11-23T17:55:20Z

Going to go ahead and merge this. Thanks again @dawitam11!

cee-chen · 2022-11-23T17:56:29Z

@miukimiu, do you mind handling the "Updated the Figma library counterpart" checklist step separately from this PR?

`eui@70.2.4` ⏩ `eui@70.4.0` - "Fixed EuiButtonGroup firing onChange twice" required changing some tests from `click` to `change` ___ ## [`70.4.0`](https://github.com/elastic/eui/tree/v70.4.0) - Updated `EuiTourStep.footerAction` type to accept `ReactNode[]` ([#6384](elastic/eui#6384)) - Vertically aligned all footer content so that `euiTourStepIndicator` is always centered ([#6384](elastic/eui#6384)) - Added `filterInCircle` glyph to `EuiIcon` ([#6385](elastic/eui#6385)) - Added `color` prop to `EuiBeacon` ([#6420](elastic/eui#6420)) - Added the `euiMaxBreakpoint` and `euiMinBreakpoint` CSS-in-JS utilities for creating min/max-width media queries ([#6431](elastic/eui#6431)) **Bug fixes** - Restores the previous match operator behaviour when the query value is split into multiple terms after analysis. ([#6409](elastic/eui#6409)) - Fixed missing slide-in animation on `EuiCollapsibleNav`s and left-side `EuiFlyout`s ([#6422](elastic/eui#6422)) - Fix bug in `EuiCard` where footer were not aligned to the bottom of the card ([#6424](elastic/eui#6424)) - Fixed multiple component media queries for consumers with custom theme breakpoints ([#6431](elastic/eui#6431)) ## [`70.3.0`](https://github.com/elastic/eui/tree/v70.3.0) - `EuiSearchBar` now automatically wraps special characters not used by query syntax in quotes ([#6356](elastic/eui#6356)) - Added `alignment` prop to `EuiBetaBadge` ([#6361](elastic/eui#6361)) - `EuiButton` now accepts `minWidth={false}` ([#6373](elastic/eui#6373)) **Bug fixes** - Fixed `EuiPageTemplate` not correctly passing the `component` prop to the inner main content wrapper. ([#6352](elastic/eui#6352)) - `EuiSkipLink` now correctly calls `onClick` even when `fallbackDestination` is invalid ([#6355](elastic/eui#6355)) - Permanently fixed `EuiModal` to not cause scroll-jumping issues on modal open ([#6360](elastic/eui#6360)) - Re-fixed `EuiPageSection` not correctly merging `contentProps.css` ([#6365](elastic/eui#6365)) - Fixed `EuiTab` not defaulting to size `m` ([#6366](elastic/eui#6366)) - Fixed the shadow sizes of `.eui-yScrollWithShadows` and `.eui-xScrollWithShadows` ([#6374](elastic/eui#6374)) - Fixed bug in `EuiCard` where the inner content in vertical cards was not growing 100% in width ([#6377](elastic/eui#6377)) - Fixed incorrect margins in `EuiSuperDatePicker` caused by `EuiFlex` CSS gap change ([#6380](elastic/eui#6380)) - Fixed visual bug in nested `EuiFlexGroup`s, where the parent `EuiFlexGroup` is responsive but a child `EuiFlexGroup` is not ([#6381](elastic/eui#6381)) **CSS-in-JS conversions** - Converted `EuiModal` to Emotion ([#6321](elastic/eui#6321)) **Fixes** - `EuiButton` no longer outputs unnecessary inline styles for `minWidth={0}` or `minWidth={false}` ([#6373](elastic/eui#6373)) - `EuiFacetButton` no longer reports type issues when passing props accepted by `EuiButton` ([#6373](elastic/eui#6373)) Co-authored-by: Constance Chen <constance.chen@elastic.co>

[EuiBeacon] Add color prop

c6f5542

Add CL entry

ac3f76b

elizabetdev reviewed Nov 21, 2022

View reviewed changes

src/components/beacon/beacon.tsx Outdated Show resolved Hide resolved

Omit color field from HTMLAttributes

002654c

dawitamene marked this pull request as ready for review November 21, 2022 17:42

cee-chen suggested changes Nov 21, 2022

View reviewed changes

src/components/beacon/beacon.styles.ts Outdated Show resolved Hide resolved

src/components/beacon/beacon.tsx Outdated Show resolved Hide resolved

dawitamene and others added 2 commits November 22, 2022 12:03

Remove color parameter from Emotion style function and return styles …

37b613f

…with individual color keys instead

Merge remote-tracking branch 'upstream/main' into beacon/color-prop

c52d847

cee-chen approved these changes Nov 22, 2022

View reviewed changes

cee-chen merged commit bf20f2a into elastic:main Nov 23, 2022

1Copenut mentioned this pull request Nov 23, 2022

Bumping EUI to v70.4.0 elastic/kibana#145888

Merged

Conversation

dawitamene commented Nov 21, 2022 • edited by cee-chen Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

General checklist

Uh oh!

kibanamachine commented Nov 21, 2022

Uh oh!

cla-checker-service bot commented Nov 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dawitamene commented Nov 21, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cee-chen commented Nov 22, 2022

Uh oh!

cee-chen left a comment

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 22, 2022

Choose a reason for hiding this comment

Uh oh!

dawitamene Nov 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cee-chen Nov 23, 2022

Choose a reason for hiding this comment

Uh oh!

kibanamachine commented Nov 22, 2022

Uh oh!

elizabetdev commented Nov 22, 2022

Uh oh!

cee-chen commented Nov 23, 2022

Uh oh!

cee-chen commented Nov 23, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dawitamene commented Nov 21, 2022 •

edited by cee-chen

Loading

cla-checker-service bot commented Nov 21, 2022 •

edited

Loading

dawitamene Nov 23, 2022 •

edited

Loading