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

FI-3261: Add markdown support to input descriptions #546

Merged
merged 22 commits into from
Dec 17, 2024

Conversation

AlyssaWang
Copy link
Collaborator

Summary

Adds Markdown support to input descriptions. Also replaces the term requirement with input.

Testing Guidance

Check to see that Markdown works properly in descriptions. See demo_group.rb for an example.

Copy link

codecov bot commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 83.09859% with 36 lines in your changes missing coverage. Please review.

Project coverage is 84.22%. Comparing base (1565a94) to head (ec23097).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
client/src/components/InputsModal/InputFields.tsx 53.84% 6 Missing ⚠️
.../src/components/InputsModal/InputCheckboxGroup.tsx 76.19% 5 Missing ⚠️
...c/components/InputsModal/InputOAuthCredentials.tsx 75.00% 5 Missing ⚠️
client/src/components/InputsModal/InputAccess.tsx 83.33% 4 Missing ⚠️
client/src/components/InputsModal/InputAuth.tsx 80.95% 4 Missing ⚠️
...lient/src/components/InputsModal/InputCombobox.tsx 80.95% 4 Missing ⚠️
...ent/src/components/InputsModal/InputRadioGroup.tsx 78.94% 4 Missing ⚠️
...src/components/InputsModal/InputSingleCheckbox.tsx 77.77% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #546      +/-   ##
==========================================
+ Coverage   84.17%   84.22%   +0.05%     
==========================================
  Files         271      272       +1     
  Lines       11589    11584       -5     
  Branches     1279     1279              
==========================================
+ Hits         9755     9757       +2     
+ Misses       1824     1817       -7     
  Partials       10       10              
Flag Coverage Δ
backend 92.59% <100.00%> (+0.01%) ⬆️
frontend 79.20% <82.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@emichaud998
Copy link
Contributor

Tried testing this out but the markdown in the input descriptions is not working as expected to me. I tried adding a newline which worked, but then trying to add a markdown header did not seem to work. I also noticed that the font is a little different from what the input descriptions normally look like. Just want to see if this is the expected behavior?
Screenshot 2024-10-24 at 4 18 57 PM

@AlyssaWang
Copy link
Collaborator Author

I think the font issue is expected but I will take a look at updating it. I'll look into the other issues as well since there have been some minor updates to the inputs modal as a whole.

@AlyssaWang
Copy link
Collaborator Author

@emichaud998 Ok I finally worked out what was going on here -- when adding the Markdown, it can't be indented or it won't be parsed properly. I've added an example in the demo group in the Patient ID description, let me know if that works!

@arscan
Copy link
Contributor

arscan commented Nov 22, 2024

@emichaud998 Ok I finally worked out what was going on here -- when adding the Markdown, it can't be indented or it won't be parsed properly. I've added an example in the demo group in the Patient ID description, let me know if that works!

Stylistically this is pretty jarring and unintuitive for a test writer, which is why in other parts we have the format_markdown method (for example). Unfortunately there isn't an obvious spot to just add this method where Inputs are defined.

@Jammjammjamm is there an easy way to add this to Inputs descriptions? Also, aside, I noticed that we forgot to wrap TestKit.description in this method, which I think we'll want to do unless its happening elsewhere.

@Jammjammjamm
Copy link
Collaborator

I think the easiest thing to do would be to handle this in the serializers. That could be a common place to fix markdown formatting rather than doing it everywhere a markdown string could be defined.

@arscan
Copy link
Contributor

arscan commented Nov 22, 2024

I opened #564 to make it so that test writers didn't need to manually unindent these. Because it is a backend thing, waiting on @Jammjammjamm to review.

@arscan
Copy link
Contributor

arscan commented Dec 2, 2024

@AlyssaWang I merged in some backend fixes so that input description markdown doesn't need to be moved all the way back to 0 indentation for it to work, but now we have some minor conflicts that need to be resolved due to the underlying main branch changing slightly. Could you take care of those and verify the functionality still is working and then we can merge this in?

@AlyssaWang
Copy link
Collaborator Author

Conflicts should be resolved -- I made a couple of changes to fix some of the Ruby lint/test issues but otherwise the same as before.

@AlyssaWang AlyssaWang merged commit 6e669c1 into main Dec 17, 2024
10 checks passed
@AlyssaWang AlyssaWang deleted the FI-3261-markdown-descriptions branch December 17, 2024 16:03
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.

4 participants