[ML] [File data visualizer] Removing scss files #178314
Conversation
jgowdyelastic
added
release_note:fix
:ml
Feature:File and Index Data Viz
|
Pinging @elastic/ml-ui (:ml) |
| @@ -84,7 +84,7 @@ export const ResultsView: FC<Props> = ({ | |||
| </EuiFlexGroup> | |||
|
|
|||
| <EuiSpacer size="m" /> | |||
| <div className="results"> | |||
| <div> | |||
There was a problem hiding this comment.
With the removal of this divs class, I imagine you can also remove the div itself, as it appears to be extraneous at the moment.
| css={{ | ||
| width: '96px', | ||
| height: '96px', | ||
| marginLeft: euiThemeVars.euiSizeXL, | ||
| marginRight: euiThemeVars.euiSizeL, | ||
| }} |
There was a problem hiding this comment.
Should this be reformatted for Emotion? Something like:
| css={{ | |
| width: '96px', | |
| height: '96px', | |
| marginLeft: euiThemeVars.euiSizeXL, | |
| marginRight: euiThemeVars.euiSizeL, | |
| }} | |
| css={css` | |
| width: 96px; | |
| height: 96px; | |
| ${logicalCSS('margin-left', euiThemeVars.euiSizeXL)} | |
| ${logicalCSS('margin-right', euiThemeVars.euiSizeL)} | |
| `} |
There was a problem hiding this comment.
Thanks for the review. I'm not sure I understand the advantage of using a template literal over an object for these styles?
Looking at the generated css, it's basically the same.
There was a problem hiding this comment.
Yeah, technically it'll result in the same outcome. I was just thinking in terms of consistency in the writing of styles. That said, it's not a deal-breaker by any means. If ya'll prefer to write your styles in a mix of formats, I'll defer to you.
There was a problem hiding this comment.
I agree consistency is better. I've switched away from using template literals in this whole PR in favour of plain objects, which read better to me.
Updated in 584db08
| @@ -56,11 +57,11 @@ export class Failures extends Component<Props, State> { | |||
| } | |||
| paddingSize="m" | |||
| > | |||
| <div className="failure-list"> | |||
| <div css={{ maxHeight: '200px', overflowY: 'auto' }}> | |||
There was a problem hiding this comment.
Same question as above. Reformat for Emotion?
| <div css={{ maxHeight: '200px', overflowY: 'auto' }}> | |
| <div css={css` | |
| ${logicalCSS('max-height', '200px')} | |
| overflow-y: auto; | |
| `}> |
| {this._renderPaginationControl()} | ||
| {this.props.failedDocs.slice(startIndex, endIndex).map(({ item, reason, doc }) => ( | ||
| <div key={item}> | ||
| <div className="error-message"> | ||
| <div css={{ color: euiThemeVars.euiColorDanger }}> |
There was a problem hiding this comment.
Same question as above. Reformat for Emotion?
| <div css={{ color: euiThemeVars.euiColorDanger }}> | |
| <div css={css` | |
| color: ${euiThemeVars.euiColorDanger}; | |
| `}> |
💚 Build Succeeded
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: |
Removes all scss files in favour of inline css.
Fixes misalignment of icons on the landing page.
Before:
After:
Note, it does leave in the scss file in the a root of the app which is used to import styles from common data visualizer components which are used in the file data visualizer for displaying stats.