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

feat(cloud): markdown: do not parse or render images in cloud envs #17714

Merged
merged 2 commits into from
Apr 13, 2020

Conversation

hoorayimhelping
Copy link
Contributor

@hoorayimhelping hoorayimhelping commented Apr 10, 2020

Closes idpe 5288

  • Creates a cloud-aware markdown renderer that behaves as it always has on OSS, but blocks image loading on cloud.
  • Refactored existing Markdown component to use the new MarkdownRender and to be renamed ScrollableMarkdown (it's wrapped in a DapperScrollbar) to avoid future confusion.
  • Made all other instances of markdown rendering use the new cloud aware renderer

Cloud behavior:

  • In editor window, show warning: We don't support images in markdown for security purposes
  • In note, show literal markdown string

OSS Behavior:

  • Render the image normally

Testing Instructions

  1. In an OSS Context:
  2. Go to a dashboard
  3. Click add note
  4. In the text editor add a markdown image ![](url.to.image/image.jpg)
  5. it should render in the preview
  6. Click save
  7. It should Shown below the cell.

  1. For a cloud context, change this line to if (CLOUD || true)
  2. Reload the dashboard you just edited
  3. The note you added should show the literal image markdown
  4. In the top right corner of the note, click the gear and click Edit Note
  5. You should see a warning saying we can't render markdown for security reasons
  6. add another markdown image ![](url.image) to test parsing new text - the same warning should pop up

Cloud

Screen Shot 2020-04-10 at 12 27 24 PM

Cloud

Screen Shot 2020-04-10 at 4 58 36 PM

OSS

Screen Shot 2020-04-10 at 4 58 04 PM

@hoorayimhelping hoorayimhelping force-pushed the bs_markdown_no_images branch 3 times, most recently from b3364a2 to 95f9bcc Compare April 13, 2020 19:07
@hoorayimhelping hoorayimhelping requested review from a team, 121watts, drdelambre and alexpaxton and removed request for a team April 13, 2020 19:09
Copy link
Contributor

@drdelambre drdelambre left a comment

Choose a reason for hiding this comment

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

super dope that we don't have to maintain whatever decides "this is an image". also.. sweet mocks

@hoorayimhelping hoorayimhelping merged commit 0d092d3 into master Apr 13, 2020
@hoorayimhelping hoorayimhelping deleted the bs_markdown_no_images branch April 13, 2020 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants