adding opt in for telemetry to start trial and upload license #22925
Conversation
💚 Build Succeeded |
… and improving a11y for read more
Mostly spacing and alignment fixes.
💚 Build Succeeded |
…_management Design cleanup
💔 Build Failed |
💚 Build Succeeded |
…agement' into bmcconaghy-telemetry_opt_in_license_management # Conflicts: # x-pack/plugins/license_management/__jest__/__snapshots__/upload_license.test.js.snap
…_management IE Fixes
💚 Build Succeeded |
…_in_license_management
💔 Build Failed |
|
retest |
💔 Build Failed |
| } | ||
|
|
||
| const readMoreButton = ( | ||
| <EuiLink onClick={() => { this.setState({ showMoreTelemetryInfo: !showMoreTelemetryInfo });}}> |
There was a problem hiding this comment.
For better readability, how about extracting this into a onClickReadMore method?
| button={readMoreButton} | ||
| isOpen={showMoreTelemetryInfo} | ||
| closePopover={this.closeReadMorePopover} | ||
| style={{ verticalAlign: 'baseline' }} |
There was a problem hiding this comment.
I don't see this style being applied in the DOM. Is it necessary?
There was a problem hiding this comment.
@cjcenizal That's applied to the trigger (button) wrapper, not the popover itself.
There was a problem hiding this comment.
Ah, of course... I should know this! :D
| closePopover={this.closeReadMorePopover} | ||
| style={{ verticalAlign: 'baseline' }} | ||
| > | ||
| <EuiText style={{ width: 240 }} > |
There was a problem hiding this comment.
Can we extract this into a CSS class? If we keep all our styles in LESS/SCSS files it will be easier to integrate them with EUI vars/mixins and to identify cases for common components/styles.
There was a problem hiding this comment.
This is how we've been handling sizing the width's of popovers (see first example). Just usually it's a div that contains the content, I just skipped the extra div and put it directly on the text container.
There was a problem hiding this comment.
I think originally we were using styles in the EUI examples to make it clearer that custom styles were in use (otherwise it would seem like the classes were something provided by EUI).
| <p> | ||
| This feature periodically sends basic feature usage statistics. | ||
| This information will not be shared outside of Elastic. | ||
| See an <EuiLink onClick={() => { this.setState({ showExample: true }); this.closeReadMorePopover(); }}>example</EuiLink> |
There was a problem hiding this comment.
Can we extract this into an onClickExample method?
| label={<span>Send basic feature usage statistics to Elastic periodically. {popover}</span>} | ||
| id="isOptingInToTelemetry" | ||
| checked={isOptingInToTelemetry} | ||
| onChange={event => { |
There was a problem hiding this comment.
Can we extract this into an onChangeOptIn method?
| </EuiTitle> | ||
| </EuiFlexItem> | ||
| </EuiFlexGroup> | ||
| <React.Fragment> |
There was a problem hiding this comment.
Can we use a named import instead?
import React, { Fragment } from 'react';
…_in_license_management
💚 Build Succeeded |
…c#22925) * adding opt in for telemetry to start trial and upload license * fixing scrolling issue with start basic modal in small browser window and improving a11y for read more * Design cleanup Mostly spacing and alignment fixes. * updating jest snapshots * IE Fixes * PR feedback
#23273) * adding opt in for telemetry to start trial and upload license * fixing scrolling issue with start basic modal in small browser window and improving a11y for read more * Design cleanup Mostly spacing and alignment fixes. * updating jest snapshots * IE Fixes * PR feedback
This changeset adds an opt in to telemetry for the start trial acknowledge modal and upload license screens for license management. It leverages code from the xpack_main plugin (this dependency is hidden from the code via a local lib file).