Skip to content
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

[Alerting] Create alert design cleanup #56929

Merged
merged 9 commits into from
Feb 12, 2020

Conversation

andreadelrio
Copy link
Contributor

@andreadelrio andreadelrio commented Feb 5, 2020

Summary

  • Fix euiComboBoxOptionsList which was being set to the back
  • Adjust spacing and add horizontal rule
  • Replace Change button with cross icon for consistency
  • Adjust labels
  • Split syntax into two sections “Select Index to query” and “Define the alert condition” for clarity
  • Adjust chart size

Before

Screen Shot 2020-02-05 at 2 35 39 PM

Group 2

Pending

When the user selects more than one index there's no space between them. @YulNaumenko can you help out with that?

add_space

Checklist

Delete any items that are not applicable to this PR.

For maintainers

- [ ] This was checked for breaking API changes and was labeled appropriately

@andreadelrio andreadelrio requested a review from a team as a code owner February 5, 2020 22:51
@andreadelrio andreadelrio added release_note:skip Skip the PR/issue when compiling release notes Feature:Alerting v7.7.0 v8.0.0 labels Feb 5, 2020
@andreadelrio
Copy link
Contributor Author

@elasticmachine merge upstream

.yarnrc Outdated
@@ -1 +1,7 @@
--ignore-workspace-root-check true
# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
# yarn lockfile v1
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that this file should be added?

Copy link
Contributor

Choose a reason for hiding this comment

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

The same for the file above yarn-1.22.0.js

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

LGTM.
But makes sure before merge that yarn specific files was removed.

@elasticmachine
Copy link
Contributor

💔 Build Failed

History

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

@elastic elastic deleted a comment from kibanamachine Feb 6, 2020
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

@andreadelrio
Copy link
Contributor Author

Added space to Index expression with help from Yuliia
image

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

@snide snide requested a review from mdefazio February 10, 2020 19:56
Copy link
Contributor

@mdefazio mdefazio left a comment

Choose a reason for hiding this comment

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

LGTM, We may want to decrease the space between expression controls a bit in the future, but that can be done with other clean ups in the future.

@andreadelrio andreadelrio removed the request for review from snide February 12, 2020 21:27
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

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

@andreadelrio andreadelrio merged commit 0faab4a into elastic:master Feb 12, 2020
gmmorris added a commit to gmmorris/kibana that referenced this pull request Feb 13, 2020
* master:
  add `absolute` option to `getUrlForApp` (elastic#57193)
  [Telemetry] Migrate public to NP (elastic#56285)
  address flaky test where instances might have different start… (elastic#57506)
  fix(NA): support legacy plugins path in plugins (elastic#57472)
  build immutable bundles for new platform plugins (elastic#53976)
  [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057)
  Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922)
  Use default spaces suffix for signals index if spaces disabled (elastic#57244)
  [Alerting] Create alert design cleanup (elastic#56929)
mbondyra added a commit to mbondyra/kibana that referenced this pull request Feb 13, 2020
* master: (22 commits)
  Use log4j pattern syntax (elastic#57433)
  [ML] Categorization field example endpoint tests (elastic#57471)
  [Lens] Filter out pinned filters from saved object of Lens (elastic#57197)
  Lens client side shim cleanup (elastic#56976)
  [Maps] do not show border color for icon in legend when border width is zero (elastic#57501)
  refactors 'data-providers' tests (elastic#57474)
  add `absolute` option to `getUrlForApp` (elastic#57193)
  [Telemetry] Migrate public to NP (elastic#56285)
  address flaky test where instances might have different start… (elastic#57506)
  fix(NA): support legacy plugins path in plugins (elastic#57472)
  build immutable bundles for new platform plugins (elastic#53976)
  [SIEM] [Detection Engine] Reject if duplicate rule_id in request payload (elastic#57057)
  Add autocomplete="off" for input type="password" to appease the scanners (elastic#56922)
  Use default spaces suffix for signals index if spaces disabled (elastic#57244)
  [Alerting] Create alert design cleanup (elastic#56929)
  Management Api - add to migration guide (elastic#56892)
  fixing maps (elastic#56706)
  [Maps] Autocomplete for custom color palettes and custom icon palettes (elastic#56446)
  [Alerting] make actionGroup name's i18n-able (elastic#57404)
  fixed flaky test (elastic#57490)
  ...

# Conflicts:
#	src/legacy/core_plugins/telemetry/public/components/__snapshots__/telemetry_form.test.js.snap
#	src/plugins/telemetry/public/components/telemetry_management_section.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:Alerting release_note:skip Skip the PR/issue when compiling release notes v7.7.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants