-
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
[Guided onboarding] Copy updates for the Security guide #143868
[Guided onboarding] Copy updates for the Security guide #143868
Conversation
Pinging @elastic/platform-onboarding (Team:Journey/Onboarding) |
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.
Config changes LGTM. Left one comment about supporting translations.
@@ -9,22 +9,20 @@ | |||
import type { GuideConfig } from '../../types'; | |||
|
|||
export const securityConfig: GuideConfig = { | |||
title: 'Get started with SIEM', | |||
title: 'Elastic Security guided setup', |
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.
Do you mind updating the strings to use i18n
as part of this PR?
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.
yes, I completely forgot about it, thanks a lot!
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.
Some minor tweaks.
src/plugins/guided_onboarding/public/constants/guides_config/security.ts
Outdated
Show resolved
Hide resolved
src/plugins/guided_onboarding/public/constants/guides_config/security.ts
Outdated
Show resolved
Hide resolved
src/plugins/guided_onboarding/public/constants/guides_config/security.ts
Outdated
Show resolved
Hide resolved
…ecurity.ts Co-authored-by: Kelly Murphy <[email protected]>
…ecurity.ts Co-authored-by: Kelly Murphy <[email protected]>
…ecurity.ts Co-authored-by: Kelly Murphy <[email protected]>
Thanks a lot for your review, @kellyemurphy! I committed your suggestions and also added a screenshot for "Step 2 manual completion". It's when the user already enabled the rules needed for the guide but they can enable more if they want. Could you please have another look? |
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.
Latest LGTM! Left one comment about a trailing .
defaultMessage: 'Load the prebuilt rules.', | ||
}), | ||
i18n.translate('guidedOnboarding.securityGuide.rulesStep.description2', { | ||
defaultMessage: 'Select the rules that you want..', |
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.
defaultMessage: 'Select the rules that you want..', | |
defaultMessage: 'Select the rules that you want.', |
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.
Two more small changes, approving with changes.
src/plugins/guided_onboarding/public/constants/guides_config/security.ts
Outdated
Show resolved
Hide resolved
src/plugins/guided_onboarding/public/constants/guides_config/security.ts
Outdated
Show resolved
Hide resolved
…ecurity.ts Co-authored-by: Kelly Murphy <[email protected]>
…ecurity.ts Co-authored-by: Kelly Murphy <[email protected]>
💚 Build Succeeded
Metrics [docs]Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
Partially addresses #139741
This PR updates the filler text with the copy from the figma designs for the Security guide.
Screenshots
Step 1
Step 2
Step 2 manual completion
Step 3
Finished state