[SIEM] Add button with link to license management in ML popup #47841

benskelker · 2019-10-10T14:52:05Z

Summary

Adds button linking to license management to the ML popup text, as requested in #462:

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
~~- [ ] Documentation was added for features that require explanation or tutorials~~
~~- [ ] Unit or functional tests were updated or added to match the most common scenarios~~
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

elasticmachine · 2019-10-10T14:52:07Z

Pinging @elastic/siem (Team:SIEM)

benskelker · 2019-10-10T14:53:48Z

@MichaelMarcialis
Please can you take a look to ensure it meets UI standards (I'm sure I missed or screwed up something)?
Thanks

elasticmachine · 2019-10-10T16:08:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 22285a7

MichaelMarcialis

Looking good! Left a couple of small comments to address before merging.

MichaelMarcialis · 2019-10-10T16:00:30Z

x-pack/legacy/plugins/siem/public/components/ml_popover/upgrade_contents.tsx

+        <EuiButton
+          href={`${chrome.getBasePath()}/app/kibana#/management/elasticsearch/license_management`}
+          iconType="popout"
+          iconSide="right"
+          target="_blank"
+        >


Since the management app is part of Kibana, I think it might be better to use managementApp for the iconType prop (as it's not an external link). Whether or not it should also have target="_blank" is debatable. I'm personally fine with it either way.

Also, I'm thinking we should remove the iconSide prop here (which will move the icon to the left side).

MichaelMarcialis · 2019-10-10T16:08:00Z

x-pack/legacy/plugins/siem/public/components/ml_popover/upgrade_contents.tsx

+        <EuiButton
+          href="https://www.elastic.co/subscriptions"
+          iconType="popout"
+          iconSide="right"
+          target="_blank"
+        >
+          {i18n.UPGRADE_BUTTON}
+        </EuiButton>
+        <EuiButton
+          href={`${chrome.getBasePath()}/app/kibana#/management/elasticsearch/license_management`}
+          iconType="popout"
+          iconSide="right"
+          target="_blank"
+        >


You'll need to wrap each of the EuiButton components in a EuiFlexItem component. You'll also want to give each EuiFlexItem a prop of grow={false}.

MichaelMarcialis · 2019-10-10T16:12:58Z

x-pack/legacy/plugins/siem/public/components/ml_popover/upgrade_contents.tsx

-      >
-        {i18n.UPGRADE_BUTTON}
-      </EuiButton>
+      <EuiFlexGroup justifyContent="spaceBetween" gutterSize="none">


Couple of changes to the props for EuiFlexGroup:

Remove justifyContent prop.

Make gutterSize prop gutterSize="s".

Add boolean prop wrap.

benskelker · 2019-10-10T17:24:21Z

@MichaelMarcialis Thanks for your help, made the corrections:

MikePaquette

LGTM, thanks!

MichaelMarcialis

Thanks for making those changes @benskelker. This looks great. Approving now so as not to block you, but before you merge, would you mind changing the managementApp icon to gear? I thought it would inherit the button colors, but I guess it doesn't. Changing to gear should fix that :)

benskelker · 2019-10-10T18:16:00Z

Sure:

spong · 2019-10-10T18:33:48Z

x-pack/plugins/translations/translations/ja-JP.json

@@ -11157,4 +11157,4 @@
    "xpack.fileUpload.fileParser.errorReadingFile": "ファイルの読み込み中にエラーが発生しました",
    "xpack.fileUpload.fileParser.noFileProvided": "エラー、ファイルが提供されていません"
  }
-}
+}


Suggested change

}

}

I think these are unintentional -- does regenerating them return the newline?

spong · 2019-10-10T18:33:53Z

x-pack/plugins/translations/translations/zh-CN.json

@@ -11323,4 +11323,4 @@
    "xpack.fileUpload.fileParser.errorReadingFile": "读取文件时出错",
    "xpack.fileUpload.fileParser.noFileProvided": "错误，未提供任何文件"
  }
-}
+}


I think these are unintentional -- does regenerating them return the newline?

Hey @spong
I regenerated them, but they didn't change. Is it a concern or can I merge?

Interesting. Judging by the history it looks like it's bounced back and forth between having a terminating new line and not. Generally speaking you want the terminating new line, so I would just revert these two files since that's the only changes.

Suggested change

}

}

Can try accepting this suggestion from within Github as well too 🙂

elasticmachine · 2019-10-10T18:36:09Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: df91e67

elasticmachine · 2019-10-10T19:32:53Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 85e88e8

elasticmachine · 2019-10-10T20:17:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 4c16339

elasticmachine · 2019-10-10T21:57:12Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: b8808fe

spong · 2019-10-10T22:02:21Z

x-pack/plugins/translations/translations/zh-CN.json

@@ -1,4 +1,4 @@
-{
+SIEM_license_button_ML_popup{


Looks like this snagged the build with the following error:

[21:11:34] Compatibility check with x-pack/plugins/translations/translations/zh-CN.json [failed] [21:11:34] → Unexpected token S in JSON at position 0 [21:11:34] Compatibility Checks [failed] [21:11:34] → Unexpected token S in JSON at position 0 ERROR Unhandled exception! ERROR SyntaxError: Unexpected token S in JSON at position 0 at JSON.parse (<anonymous>) at parse (/var/lib/jenkins/workspace/elastic+kibana+pipeline-pull-request/kibana/src/dev/i18n/integrate_locale_files.ts:183:34)

elasticmachine · 2019-10-11T10:43:22Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: d56a2f6

benskelker · 2019-10-11T10:52:12Z

retest

elasticmachine · 2019-10-11T12:18:59Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d56a2f6

…c#47841) * Add button with link to license management in ML popup * Corrections to button styling * Updated button icon * Update snapshots * Correcting json translation files * Fix zh_CN json file * And now fixing the translation file properly

#48207) * Add button with link to license management in ML popup * Corrections to button styling * Updated button icon * Update snapshots * Correcting json translation files * Fix zh_CN json file * And now fixing the translation file properly

Add button with link to license management in ML popup

22285a7

benskelker added Team:SIEM release_note:skip Skip the PR/issue when compiling release notes v7.5.0 labels Oct 10, 2019

benskelker requested review from spong, MichaelMarcialis and MikePaquette October 10, 2019 14:52

MichaelMarcialis requested changes Oct 10, 2019

View reviewed changes

Corrections to button styling

df91e67

MikePaquette approved these changes Oct 10, 2019

View reviewed changes

MichaelMarcialis approved these changes Oct 10, 2019

View reviewed changes

Updated button icon

85e88e8

spong reviewed Oct 10, 2019

View reviewed changes

Update snapshots

4c16339

Correcting json translation files

b8808fe

spong reviewed Oct 10, 2019

View reviewed changes

Ben Skelker added 2 commits October 11, 2019 09:52

Fix zh_CN json file

2cda215

And now fixing the translation file properly

d56a2f6

benskelker merged commit f591b59 into elastic:master Oct 11, 2019

benskelker deleted the SIEM_license_button_ML_popup branch October 11, 2019 12:33

benskelker mentioned this pull request Oct 15, 2019

[7.x] [SIEM] Add button with link to license management in ML popup (#47841) #48207

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SIEM] Add button with link to license management in ML popup #47841

[SIEM] Add button with link to license management in ML popup #47841

benskelker commented Oct 10, 2019 •

edited

Loading

elasticmachine commented Oct 10, 2019

benskelker commented Oct 10, 2019

elasticmachine commented Oct 10, 2019

MichaelMarcialis left a comment

MichaelMarcialis Oct 10, 2019

MichaelMarcialis Oct 10, 2019

MichaelMarcialis Oct 10, 2019

benskelker commented Oct 10, 2019 •

edited

Loading

MikePaquette left a comment

MichaelMarcialis left a comment

benskelker commented Oct 10, 2019

spong Oct 10, 2019 •

edited

Loading

spong Oct 10, 2019

benskelker Oct 10, 2019

spong Oct 10, 2019

spong Oct 10, 2019

elasticmachine commented Oct 10, 2019

elasticmachine commented Oct 10, 2019

elasticmachine commented Oct 10, 2019

elasticmachine commented Oct 10, 2019

spong Oct 10, 2019

elasticmachine commented Oct 11, 2019

benskelker commented Oct 11, 2019

elasticmachine commented Oct 11, 2019

[SIEM] Add button with link to license management in ML popup #47841

[SIEM] Add button with link to license management in ML popup #47841

Conversation

benskelker commented Oct 10, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Oct 10, 2019

benskelker commented Oct 10, 2019

elasticmachine commented Oct 10, 2019

💚 Build Succeeded

MichaelMarcialis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benskelker commented Oct 10, 2019 • edited Loading

MikePaquette left a comment

Choose a reason for hiding this comment

MichaelMarcialis left a comment

Choose a reason for hiding this comment

benskelker commented Oct 10, 2019

spong Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Oct 10, 2019

💚 Build Succeeded

elasticmachine commented Oct 10, 2019

💔 Build Failed

elasticmachine commented Oct 10, 2019

💚 Build Succeeded

elasticmachine commented Oct 10, 2019

💔 Build Failed

Choose a reason for hiding this comment

elasticmachine commented Oct 11, 2019

💔 Build Failed

benskelker commented Oct 11, 2019

elasticmachine commented Oct 11, 2019

💚 Build Succeeded

benskelker commented Oct 10, 2019 •

edited

Loading

benskelker commented Oct 10, 2019 •

edited

Loading

spong Oct 10, 2019 •

edited

Loading