-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
Btsymbala/registered av #81910
Btsymbala/registered av #81910
Changes from 20 commits
12f6191
0501c1d
56f8f44
7e216d7
0b4a7eb
62db297
509aace
25d0d48
4734757
fbff194
ecfc5b3
7a1d007
ee19c19
7289449
8a07958
9e1fed6
623a91b
df503b5
15c0265
98360a8
41b9750
f1b02f3
773204f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
|
||
export type Linux = 'linux'; | ||
export type MacOS = 'macos'; | ||
export type Windows = 'windows'; | ||
export type OperatingSystem = Linux | MacOS | Windows; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,60 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License; | ||
* you may not use this file except in compliance with the Elastic License. | ||
*/ | ||
import React from 'react'; | ||
import { ThemeProvider } from 'styled-components'; | ||
import { storiesOf, addDecorator } from '@storybook/react'; | ||
import euiLightVars from '@elastic/eui/dist/eui_theme_light.json'; | ||
import { EuiCheckbox, EuiSpacer, EuiSwitch, EuiText } from '@elastic/eui'; | ||
|
||
import { ConfigForm } from '.'; | ||
|
||
addDecorator((storyFn) => ( | ||
<ThemeProvider theme={() => ({ eui: euiLightVars, darkMode: false })}>{storyFn()}</ThemeProvider> | ||
)); | ||
|
||
storiesOf('PolicyDetails/ConfigForm', module) | ||
.add('One OS', () => { | ||
return ( | ||
<ConfigForm type="Type 1" supportedOss={['windows']}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we use the os enums you made here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you refer to os.ts file then it contains only type definitions, unfortunately literal types can not be used as keys/symbols directly. I plan to later revisit this and add some constant to keep the keys. |
||
{'Some content'} | ||
</ConfigForm> | ||
); | ||
}) | ||
.add('Multiple OSs', () => { | ||
return ( | ||
<ConfigForm type="Type 1" supportedOss={['windows', 'macos', 'linux']}> | ||
{'Some content'} | ||
</ConfigForm> | ||
); | ||
}) | ||
.add('Complex content', () => { | ||
return ( | ||
<ConfigForm type="Type 1" supportedOss={['macos', 'linux']}> | ||
<EuiText> | ||
{'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore ' + | ||
'et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut ' + | ||
'aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum ' + | ||
'dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia ' + | ||
'deserunt mollit anim id est laborum.'} | ||
</EuiText> | ||
<EuiSpacer size="s" /> | ||
<EuiSwitch label={'Switch'} checked={true} onChange={() => {}} /> | ||
<EuiSpacer size="s" /> | ||
<EuiCheckbox id="1" label={'Checkbox 1'} checked={false} onChange={() => {}} /> | ||
<EuiCheckbox id="2" label={'Checkbox 2'} checked={true} onChange={() => {}} /> | ||
<EuiCheckbox id="3" label={'Checkbox 3'} checked={true} onChange={() => {}} /> | ||
</ConfigForm> | ||
); | ||
}) | ||
.add('Right corner content', () => { | ||
const toggle = <EuiSwitch label={'Switch'} checked={true} onChange={() => {}} />; | ||
|
||
return ( | ||
<ConfigForm type="Type 1" supportedOss={['linux']} rightCorner={toggle}> | ||
{'Some content'} | ||
</ConfigForm> | ||
); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason why you created a separate action for antivirus registration specifically? If I am reading this right, it seems like the
antivirus_registration
is just another key within the policy config. The actionuserChangedPolicyConfig
should be able to do the same thing. ex) https://github.com/elastic/kibana/blob/master/x-pack/plugins/security_solution/public/management/pages/policy/view/policy_forms/protections/malware.tsx#L53There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will bring this up during next week office hours. General thought was to avoid components being too smart and knowing how to update properties in policy config and rather keep the logic out (specifically move it to redux). What are your thoughts on this?
I noticed that due to this logic living in component we clone policy twice and have some ts-ignore directives because the code violates immutability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh okie, i can see the value in moving logic into redux, i guess im just wondering if its possible to make it more robust since i feel like policy details will only get bigger in the future?