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.

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

[Security Solution][All] Replace old markdown renderer with the new one #80301

Merged
merged 6 commits into from
Oct 21, 2020

Conversation

cnasikas
Copy link
Member

@cnasikas cnasikas commented Oct 13, 2020

Summary

This PR introduced the new EuiMarkdownEditor for cases. Some components in the Security Solution app still uses the old markdown editor. This PR removes the old markdown and replace it with the new one.

Note: Markdown URLs are enabled on the case widget on the overview page. Before this PR there were not enabled (rendered).

Screenshot 2020-10-13 at 12 00 13 PM

Screenshot 2020-10-13 at 12 01 31 PM

Checklist

Delete any items that are not applicable to this PR.

For maintainers

@cnasikas cnasikas added Team:SIEM v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.11.0 Team:Detections and Resp Security Detection Response Team Team:Threat Hunting Security Solution Threat Hunting Team labels Oct 13, 2020
@cnasikas cnasikas requested review from a team as code owners October 13, 2020 09:02
@cnasikas cnasikas self-assigned this Oct 13, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/siem (Team:SIEM)

@@ -136,7 +136,7 @@ const StepAboutRuleToggleDetailsComponent: React.FC<StepPanelProps> = ({
maxHeight={aboutPanelHeight}
className="eui-yScrollWithShadows"
>
<Markdown raw={stepDataDetails.note} />
<MarkdownRenderer>{stepDataDetails.note}</MarkdownRenderer>
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 thank you!

Copy link
Contributor

@yctercero yctercero left a comment

Choose a reason for hiding this comment

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

LGTM (Note - just focused on use in rules, not cases)! Pulled down and tested markdown on rule creation, made sure it shows up in rule details and that a new note is appended to a timeline opened from an alert where the rule has an investigation guide 👍 Thanks for updating this!

Copy link
Contributor

@stephmilovic stephmilovic left a comment

Choose a reason for hiding this comment

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

Code review and manual testing both LGTM. Thanks @cnasikas !!

@cnasikas cnasikas added the Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. label Oct 16, 2020
@cnasikas
Copy link
Member Author

@elasticmachine merge upstream

@cnasikas cnasikas force-pushed the replace_markdown branch 2 times, most recently from 2ac9079 to 718630e Compare October 20, 2020 08:20
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

@kbn/optimizer bundle module count

id before after diff
securitySolution 2057 2060 +3

async chunks size

id before after diff
securitySolution 8.1MB 8.1MB -2.7KB

History

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

<EuiLink
href={disableLinks ? undefined : href}
data-test-subj="markdown-link"
rel={`${REL_NOFOLLOW}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consider dropping the extraneous template, i.e.

rel={REL_NOFOLLOW}

Copy link
Contributor

@andrew-goldstein andrew-goldstein left a comment

Choose a reason for hiding this comment

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

Thanks for migrating the rest of the Security solution to the EUI Markdown formatter and renderer @cnasikas! 🙏

Desk tested:

  • global timeline notes
  • inline timeline notes
  • case notes
  • rendering case descriptions w/links in the Overview
  • external link rendering
  • timeline link rendering
  • timeline Analyze event-link rendering
  • x-browser markdown formatting (the example below is from FF):
    formatting-firefox

LGTM 🚀 📓 ✏️

@cnasikas cnasikas merged commit ad2fe78 into elastic:master Oct 21, 2020
@cnasikas cnasikas deleted the replace_markdown branch October 21, 2020 07:32
cnasikas added a commit to cnasikas/kibana that referenced this pull request Oct 21, 2020
jloleysens added a commit to jloleysens/kibana that referenced this pull request Oct 21, 2020
…arm-phase-to-formlib

* 'master' of github.com:elastic/kibana: (55 commits)
  [UX] Fix map color variance and apply proper filter for extended stats (elastic#81106)
  [User Experience] Use EuiSelect for percentiles instead of SuperSelect (elastic#81082)
  [DOCS] Add link for monitoring ssl settings (elastic#81057)
  [test] Await loading indicator in monitoring test (elastic#81279)
  [ILM] Minor copy and link additions to cloud CTA for cold phase (elastic#80512)
  [Mappings editor] Add scaled_float and date_range comp integration tests (elastic#81287)
  [Discover] Deangularize context.app (elastic#80851)
  [O11y Overview] Add code to display/hide UX section when appropriate (elastic#80873)
  [Discover] Extend DiscoverNoResults component to show different message on error (elastic#79671)
  Fix tagcloud word overlapping (elastic#81161)
  [Security Solution] Fixes flaky test rules (elastic#81040)
  Changed the code to avoid tech debt with hacky solutions after receiving comments on EUI issue reported about this problem. (elastic#81183)
  [Security Solution][All] Replace old markdown renderer with the new one (elastic#80301)
  Add namespaced version of the API call (elastic#81278)
  [ML] Data Frame Analytics: Fix race condition and support for feature influence legacy format. (elastic#81123)
  [Fleet] Fix POLICY_CHANGE action creation for new policy (elastic#81236)
  [Security Solution][Endpoint][Admin] Malware user notification checkbox (elastic#78084)
  [SecuritySolution][Unit Tests] - fix flakey unit test (elastic#81239)
  skip flaky suite (elastic#81264)
  [Maps] fix top-level Map page is called 'Kibana' (elastic#81238)
  ...

# Conflicts:
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/hot_phase/hot_phase.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/shared/forcemerge_field.tsx
#	x-pack/plugins/index_lifecycle_management/public/application/sections/edit_policy/components/phases/warm_phase.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release_note:skip Skip the PR/issue when compiling release notes Team:Detections and Resp Security Detection Response Team Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:SIEM Team:Threat Hunting Security Solution Threat Hunting Team v7.11.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants