-
Notifications
You must be signed in to change notification settings - Fork 841
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
[Emotion] Convert EuiTextArea, EuiSelect, and EuiSuperSelect #7812
Merged
cee-chen
merged 16 commits into
elastic:emotion/forms
from
cee-chen:emotion/textarea-selects
Jun 6, 2024
Merged
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
68262c1
Merge branch 'main' into emotion/forms
cee-chen e93f9c1
Convert EuiTextArea to Emotion
cee-chen 75157f0
[EuiSelect] Remove unnecessary vendor prefixes
cee-chen 30609ff
Convert EuiSelect to Emotion
cee-chen e4bd38d
[EuiSelect] Misc cleanup
cee-chen 838e4ce
[EuiSelect] Misc Storybook cleanup
cee-chen e77c3df
[EuiSuperSelect] Convert initial listbox CSS
cee-chen 4daa796
[EuiSuperSelect] Convert individual item/option styles
cee-chen f9eea31
[EuiSuperSelect] Convert form control/button to Emotion
cee-chen 3f9c939
[EuiSuperSelect] Delete Sass files
cee-chen 88d025b
Update downstream snapshots
cee-chen b8201ba
changelog
cee-chen 5a0ebfb
Update VRT screenshots
cee-chen 9b14b6e
[EuiTextArea] Fix missed CSS conversion that VRT tests caught 🎉
cee-chen 93393f8
[PR feedback] Typing
cee-chen c2f14e3
[PR feedback] Explicitly call `euiFontSize('s')`
cee-chen File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
9 changes: 0 additions & 9 deletions
9
packages/eui/src/components/form/super_select/_super_select.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +0,0 @@ | ||
.euiSuperSelect__item { | ||
@include euiFontSizeS; | ||
@include euiInteractiveStates; | ||
padding: $euiSizeS; | ||
} | ||
|
||
.euiSuperSelect__item--hasDividers:not(:last-of-type) { | ||
border-bottom: $euiBorderThin; | ||
} | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
55 changes: 55 additions & 0 deletions
55
packages/eui/src/components/form/super_select/super_select_item.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
/* | ||
* 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 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 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
import React, { FunctionComponent, ComponentProps, ReactNode } from 'react'; | ||
import classNames from 'classnames'; | ||
|
||
import { useEuiMemoizedStyles } from '../../../services'; | ||
import { EuiContextMenuItem } from '../../context_menu'; | ||
import { euiSuperSelectItemStyles } from './super_select.styles'; | ||
|
||
// Type exposed to consumers via API | ||
export interface EuiSuperSelectOption<T> { | ||
value: NonNullable<T>; | ||
inputDisplay?: ReactNode; | ||
dropdownDisplay?: ReactNode; | ||
disabled?: boolean; | ||
'data-test-subj'?: string; | ||
} | ||
|
||
// Actual props used by below component, transmogged by parent EuiSuperSelect | ||
// from consumer props to internal EUI props | ||
type EuiSuperSelectItemProps = ComponentProps<typeof EuiContextMenuItem> & { | ||
hasDividers?: boolean; | ||
}; | ||
|
||
// Internal subcomponent util, primarily for easier usage of hooks | ||
cee-chen marked this conversation as resolved.
Show resolved
Hide resolved
|
||
export const EuiSuperSelectItem: FunctionComponent<EuiSuperSelectItemProps> = ({ | ||
children, | ||
className, | ||
hasDividers, | ||
...rest | ||
}) => { | ||
const classes = classNames('euiSuperSelect__item', className); | ||
const styles = useEuiMemoizedStyles(euiSuperSelectItemStyles); | ||
const cssStyles = [ | ||
styles.euiSuperSelect__item, | ||
hasDividers && styles.hasDividers, | ||
]; | ||
|
||
return ( | ||
<EuiContextMenuItem | ||
css={cssStyles} | ||
className={classes} | ||
role="option" | ||
{...rest} | ||
> | ||
{children} | ||
</EuiContextMenuItem> | ||
); | ||
}; |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not sure if it's intended, but I think this mixin is missing in the emotion version?
I'm seeing differences due to that in the option content height (
line-height
specifically is different now)super_select_emotion_conversion.mov
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.
Shoot, I did totally miss this! 🤦 Thank you so much for catching it!
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.
Actually, it looks like the new Emotion CSS-in-JS
euiFontSize(euiThemeContext, 's')
changes the line height to be smaller anyway (Caroline intentionally changed this as part of Emotion work) so I think the reduced height would have been an intentional change in any case. (and IMO, it looks pretty good!)I'm going to go ahead and explicitly add it in any case because it's better to be clear than not clear 🤷 c2f14e3
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.
Also just mentioning for my own memory -
@include euiInteractiveStates
didn't appear to do anything / was already handled byEuiContextMenuItem
, and can be safely removed