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.

Sign up for GitHub

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

Jump to bottom

[SIEM] Improves map configuration text on Network page #52469

Merged
merged 7 commits into from
Dec 11, 2019
Merged

[SIEM] Improves map configuration text on Network page #52469

merged 7 commits into from
Dec 11, 2019

Conversation

benskelker
Copy link
Contributor

@benskelker benskelker commented Dec 8, 2019 •
edited
Loading

Summary

Tries to improve the UI text for configuring map data on the Network page, as per #52312.

Screenshot 2019-12-10 at 12 04 47

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

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

@benskelker benskelker added Team:SIEM release_note:skip Skip the PR/issue when compiling release notes v7.6.0 labels Dec 8, 2019
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@benskelker
Copy link
Contributor Author

@spong and @MichaelMarcialis

If you have any suggestions on how to shorten the text, that would be great - thanks.

@benskelker benskelker changed the title [Siem] Improves map configuration text on Network page [SIEM] Improves map configuration text on Network page Dec 8, 2019
@spong spong mentioned this pull request Dec 9, 2019
Closed
spong
spong approved these changes Dec 9, 2019
View reviewed changes
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

Checked out, verified updated text is present and that all links are functioning. LGTM 👍

Thanks for this clarification @benskelker!

@benskelker
Copy link
Contributor Author

@spong - updated to try and make it clearer as per your comments on #739. For simplicity, and because it is well documented, I removed the ECS requirement.
Let me know what you think. Thanks.

spong
spong approved these changes Dec 10, 2019
View reviewed changes
Copy link
Member

@spong spong left a comment

Choose a reason for hiding this comment

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

@spong - updated to try and make it clearer as per your comments on #739. For simplicity, and because it is well documented, I removed the ECS requirement.
Let me know what you think. Thanks.

I think this spells out the requirements perfectly. The user should now have a clear path forward for what they need to do when seeing this message now. Thanks for clarifying @benskelker!

@benskelker
Copy link
Contributor Author

@MichaelMarcialis can you give this PR a quick review as well in case I've mucked up any of the attributes. Thanks

MichaelMarcialis
MichaelMarcialis approved these changes Dec 11, 2019
View reviewed changes
Copy link
Contributor

@MichaelMarcialis MichaelMarcialis left a comment

Choose a reason for hiding this comment

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

This all looks good from my perspective. Thanks, Ben!

@benskelker
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

History

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

@benskelker benskelker merged commit 6f79046 into elastic:master Dec 11, 2019
@benskelker benskelker deleted the SIEM_improved_map_requirements_message branch December 11, 2019 15:56
@benskelker benskelker mentioned this pull request Dec 11, 2019
Merged
benskelker added a commit that referenced this pull request Dec 11, 2019
@benskelker
[SIEM] Improves map configuration text on Network page (#52469) (#52766)
57303be 
* updates SIEM network page maps conf message

* corrects link atts

* updated message

* updated message again

* finally

* updates after feedback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichaelMarcialis MichaelMarcialis MichaelMarcialis approved these changes

@spong spong spong approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:SIEM v7.6.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@benskelker @elasticmachine @spong @MichaelMarcialis