Skip to content

Conversation

@ecezalp
Copy link
Contributor

@ecezalp ecezalp commented Feb 25, 2021

Summary

Types the mock themes in tests as EuiTheme.
Replaces the ThemeProvider (from styled-components) with EuiThemeProvider in app.js.
Closes elastic/security-team#866.

Checklist

@ecezalp ecezalp added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes labels Feb 25, 2021
@ecezalp ecezalp requested a review from rylnd February 25, 2021 16:34
@ecezalp ecezalp self-assigned this Feb 25, 2021
@ecezalp ecezalp requested a review from a team as a code owner February 25, 2021 16:34
@ecezalp ecezalp requested a review from rylnd March 3, 2021 15:34
@ecezalp
Copy link
Contributor Author

ecezalp commented Mar 3, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

merge conflict between base and head

Closes elastic/security-team#866.
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
securitySolution 7.8MB 7.8MB -556.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
securitySolution 237.0KB 237.2KB +238.0B

History

  • 💔 Build #110774 failed 41c972091e7e61c8349b20f27cbc5d5db96e95b3
  • 💚 Build #109457 succeeded db8d98c6dfe2f46381140c893400739ba4f5656e

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @ecezalp

Copy link
Contributor

@rylnd rylnd left a comment

Choose a reason for hiding this comment

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

Changes LGTM, thank you. I had a few questions and one nit, but nothing preventing this from being merged.

} from '../../../../../common/detection_engine/schemas/response/rules_schema.mocks';
import { useRuleAsync } from '../../../../detections/containers/detection_engine/rules/use_rule_async';
import { AlertData } from '../types';
import { getMockTheme } from '../../../lib/kibana/kibana_react.mock';
Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: I prefer to keep these relative imports roughly organized by distance

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have a neat way of doing so by pressing a few keys? Would be super interested in that!!!

euiColorLightestShade: '#ece',
euiColorPrimary: '#ece',
euiFontWeightSemiBold: 'bold',
euiFontWeightSemiBold: 1,
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

euiFontWeightSemiBold is a number! It's probably 400 or so in the light / dark theme

eui: { euiSizeS: '10px', euiLineHeight: '20px', euiBreakpoints: { s: '10px' }, euiSize: '10px' },
};
const mockTheme = getMockTheme({
eui: { euiSizeS: '10px', euiLineHeight: 10, euiBreakpoints: { s: '10px' }, euiSize: '10px' },
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto on the change here; does euiLineHeight simply need a value to prevent the test from blowing up?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

@ecezalp ecezalp merged commit 3d374e2 into elastic:master Mar 4, 2021
@ecezalp ecezalp deleted the mock-theme branch March 4, 2021 15:31
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 4, 2021
* master: (107 commits)
  [Logs UI] Fix log stream data fetching (elastic#93201)
  [App Search] Added relevance tuning search preview (elastic#93054)
  [Canvas] Fix reports embeddables (elastic#93482)
  [ILM] Added new functional test in ILM for creating a new policy (elastic#92936)
  Remove direct dependency on statehood package (elastic#93592)
  [Maps] Track tile loading status (elastic#91585)
  [Discover][Doc] Improve main documentation (elastic#91854)
  [Upgrade Assistant] Disable UA and add prompt (elastic#92834)
  [Snapshot Restore] Remove cloud validation for slm policy (elastic#93609)
  [Maps] Support GeometryCollections in GeoJson upload (elastic#93507)
  [XY Charts] fix partial histogram endzones annotations (elastic#93091)
  [Core] Simplify context typings (elastic#93579)
  [Alerting] Improving health status check (elastic#93282)
  [Discover] Restore context documentation (elastic#90784)
  [core-docs] Edits core-intro section for the new docs system (elastic#93540)
  add missed codeowners (elastic#89714)
  fetch node labels via script execution (elastic#93225)
  [Security Solution] Adds getMockTheme function (elastic#92840)
  Sort dependencies in package.json correctly (elastic#93590)
  [Bug] missing timepicker:quickRanges migration (elastic#93409)
  ...
@kibanamachine kibanamachine added the backport missing Added to PRs automatically when the are determined to be missing a backport. label Mar 8, 2021
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 92840 or prevent reminders by adding the backport:skip label.

2 similar comments
@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 92840 or prevent reminders by adding the backport:skip label.

@kibanamachine
Copy link
Contributor

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 92840 or prevent reminders by adding the backport:skip label.

@ecezalp ecezalp added v7.12 and removed backport missing Added to PRs automatically when the are determined to be missing a backport. labels Mar 10, 2021
ecezalp added a commit to ecezalp/kibana that referenced this pull request Mar 10, 2021
Closes elastic/security-team#866.
# Conflicts:
#	x-pack/plugins/security_solution/public/detections/components/rules/query_preview/index.test.tsx
ecezalp added a commit to ecezalp/kibana that referenced this pull request Mar 10, 2021
ecezalp added a commit that referenced this pull request Mar 10, 2021
Closes elastic/security-team#866.
# Conflicts:
#	x-pack/plugins/security_solution/public/detections/components/rules/query_preview/index.test.tsx
ecezalp added a commit that referenced this pull request Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release_note:skip Skip the PR/issue when compiling release notes v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants