Make markdown show - Githubissues

PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.

GNU Affero General Public License v3.0

35 stars 89 forks source link

Make markdown show #1743

Closed czhou578 closed 3 weeks ago

czhou578 commented 3 weeks ago

Description

Fixes #1703 Please provide a summary of the changes and which issue is fixed. Please include relevant motivation and context.

Changes

This PR ensure that the prompt in the Analysis.jsx file is correctly displayed once the user clicks "Show Prompt" on the UI.

Screenshots

Tests

No new tests were added.

czhou578 commented 3 weeks ago

@anth-volk, I'm not sure if this is how you want the text to be displayed in markdown. Is this how its supposed to be?

anth-volk commented 3 weeks ago

Funky seeing our 35th president editing his policy provisions on PolicyEngine haha.

It looks like one of the asterisks in the first line (in "I'm") is causing the strange display. Could you check into either fiddling with the settings so that that doesn't happen (the relevant package is CodeMirror on NPM, I believe) or somehow sanitizing it?

Also, could you wrap the text?

czhou578 commented 3 weeks ago

Does this work?

anth-volk commented 3 weeks ago

That looks good!

anth-volk commented 3 weeks ago

Looks like there's a test failing. Since I wrote that, I'll look into it either later today or tomorrow.

MaxGhenis commented 3 weeks ago

Can we please fix this before 9am ET tomorrow? @nikhilwoodruff could you tag in if needed?

anth-volk commented 3 weeks ago

I've just spent an hour or more on this and still don't understand why this test is failing - it's as if, when run, Jest clips all code below line 30 in the CodeMirror component, even though it's absolutely visible. I'm going to try to fix this soon, but if I can't find a resolution within the next 15-20 minutes, it'd be best to disable these tests.

anth-volk commented 3 weeks ago

Actually, I think I have a workaround for this

nikhilwoodruff commented 3 weeks ago

Sorry I didn't see this! Just got it working in #1754 but this looks like a nicer solution

nikhilwoodruff commented 3 weeks ago

@anth-volk- the tests are passing fine on my PR oddly

nikhilwoodruff commented 3 weeks ago

Thanks @czhou578 and sorry for stealing your PR :) - I think #1754 can get this over the line. Just added your line wrapping extension to it.

nikhilwoodruff commented 3 weeks ago

OK so I spoke too soon

anth-volk commented 3 weeks ago

Just got the tests working

anth-volk commented 3 weeks ago

It does look good. Once these pass, I'll merge

anth-volk commented 3 weeks ago

And we're good