-
Notifications
You must be signed in to change notification settings - Fork 842
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
Add typedefs for EuiAvatar component and color service #1120
Conversation
@stacey-gammon Do you mind just glancing over these TS defs? Our resident TS expert, @chandlerprall is out for the next week. |
src/services/color/index.d.ts
Outdated
type rgbDef = [number, number, number]; | ||
|
||
export const hexToRbg: (hex: string) => rgbDef; | ||
export const rbgToHex: (rbg: string) => string; |
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.
(rbg: rgbDef) => string;
?
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.
If I understand the function correctly (https://github.com/legrego/eui/blob/typedef-updates/src/services/color/rgb_to_hex.js#L2), rbgToHex
accepts a string, rather than an array of numbers.
I did transpose rgb
into rbg
by mistake, so I'll fix that now.
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.
Minus the one comment, lgtm. Did not pull down though and verify, just gave a quick glance.
…19497) This replaces the existing Modal with a smaller Popover which is less intrusive. The popover also features a search bar for finding the desired Space when there are 8 or more Spaces to choose from. ### Details When there are less than 8 spaces available, the selector will render a simple list of spaces. When there are >= 8 spaces available, the selector will also render a search bar to let users search for their space. ### Prerequisites - [x] Merge #18862 into `spaces-phase-1` ### Known Issues - elastic/eui#1043 (fixed in `v3.2.0`) - elastic/eui#1052 (fixed in `v3.2.1`) - Missing typdefs (not a blocker to merge): elastic/eui#1120
Thanks for the review @stacey-gammon! @cchaos any objections to merging this? |
Nope, go for it! |
This PR adds typedefs for the following:
EuiAvatar
componentcolor
service