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

Fix typography for submission diff #3673

Merged
merged 2 commits into from
Dec 13, 2023
Merged

Fix typography for submission diff #3673

merged 2 commits into from
Dec 13, 2023

Conversation

theskumar
Copy link
Member

@theskumar theskumar commented Nov 30, 2023

  • The numbers don't break
  • make the heading consistent, everything gets a h4
  • Update the color and style for added/deleted text

Screenshot 2023-11-30 at 10  26 31@2x

closes #3671

- The numbers don't break
- make the heading consistent, everything gets a h4
- Update the color and style for added/deleted text

closes #3671
@theskumar theskumar self-assigned this Nov 30, 2023
@theskumar
Copy link
Member Author

I've able to get another version of compare view working with rich text display. See some of the screenshots below. If there is enough interest, after we merge this, I'll polish this up a bit and send a pull request. It uses html-diff==0.4.1

The styling is borrowed from the Github's markdown diff preview.

Screenshot 2023-11-30 at 12  23 20@2x

Screenshot 2023-11-30 at 12  26 46@2x

Copy link
Contributor

@wes-otf wes-otf left a comment

Choose a reason for hiding this comment

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

Looks/works great! Especially the Proposal Infomation (Old)/(New) is a super useful addition.

@wes-otf
Copy link
Contributor

wes-otf commented Nov 30, 2023

Two small things after more testing not mentioned in the original issue:

  1. Ordered lists get turned to unordered lists and new lines look to be ignored sometimes in Rich Text fields (ie. "Here are some bullets:", "Now for some numbering:" & "Here's a link:").
  2. Links & their changes are not shown.

In the following screenshots the Open Tech Fund link was swapped from https://opentech.fund to https://www.opentech.fund but the compare shows this as unchanged.

The application w/ Rich Text & Markdown:


Screenshot 2023-11-30 at 11 05 53 AM

The compare view:


Screenshot 2023-11-30 at 11 05 21 AM

@frjo
Copy link
Contributor

frjo commented Nov 30, 2023

@wes-otf Good point on the links, we should add some code to handle them.

The submission contains large amount of HTML due to the RichTextEditor and that many people paste in text straight from Microsoft Word etc. To diff the HTML directly is not workable.

What Hypha instead does is to strip all the HTML and compare that instead. Then the code tries to reassemble the text to show it in some resemblance to the original.

This works but is far from optimal. We just discussed today that it might be worth testing to convert the text to markdown instead and compare that.

@theskumar
Copy link
Member Author

We just discussed today that it might be worth testing to convert the text to markdown instead and compare that.

After the call, I spent some more time with it. See my experiment with diffing the html itself and displaying it to user here #3673 (comment)

If the rich text support is needed, this might be the best way to go.

@wes-otf
Copy link
Contributor

wes-otf commented Dec 1, 2023

I think that would make a lot of sense. Would definitely be interested in your HTML diff @theskumar. Maybe we could do a beta for something like that similarly to the submission view to gauge interest.

@frjo frjo added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Status: Needs testing Tickets that need testing/qa Type: Patch Mini change, used in release drafter labels Dec 5, 2023
@frjo frjo merged commit d6a9fa5 into main Dec 13, 2023
10 checks passed
@theskumar theskumar deleted the fix/compare-page-layout branch December 18, 2023 09:17
wes-otf pushed a commit that referenced this pull request May 7, 2024
- The numbers don't break
- make the heading consistent, everything gets a h4
- Update the color and style for added/deleted text


closes #3671
wes-otf pushed a commit that referenced this pull request May 8, 2024
- The numbers don't break
- make the heading consistent, everything gets a h4
- Update the color and style for added/deleted text


closes #3671
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs testing Tickets that need testing/qa Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Compare view formatting is sometimes confusing to read
3 participants