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.

Sign up for GitHub

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

Jump to bottom

Fix math and mermaid rendering bugs #24049

Merged
merged 7 commits into from
Apr 17, 2023
Merged

Fix math and mermaid rendering bugs #24049

merged 7 commits into from
Apr 17, 2023

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Apr 11, 2023 •
edited
Loading

  1. Fix multiple error display for math and mermaid:

err

  1. Fix height calculation of certain mermaid diagrams by reading the iframe inner height from it's document instead of parsing it from SVG:

Before:
Screenshot 2023-04-11 at 11 56 27

After:
Screenshot 2023-04-11 at 11 56 35

  1. Refactor error handling to a common function
  2. Rename to renderAsciicast for consistency
  3. Improve mermaid loading sequence

Note: I did try securityLevel: 'sandbox' to make mermaid output a iframe directly, but that showed a bug in mermaid where the iframe style height was set incorrectly. Opened mermaid-js/mermaid#4289 for this.

@silverwind silverwind added this to the 1.20.0 milestone Apr 11, 2023
@silverwind silverwind added the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Apr 11, 2023
@silverwind silverwind changed the title Fix multiple markdown rendering bugs Fix math/mermaid rendering bugs Apr 11, 2023
@silverwind silverwind changed the title Fix math/mermaid rendering bugs Fix math and mermaid rendering bugs Apr 11, 2023
@silverwind
Copy link
Member Author

I've also improved the mermaid loading sequence where previously it would sometimes flash the iframe background when many diagrams were present on the page. The sandbox attribute was removed because having both allow-scripts and allow-same-origin (which is necessary to measure dom height) is equivalent to no sandbox.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Apr 12, 2023
@silverwind silverwind removed the outdated/backport/v1.19 This PR should be backported to Gitea 1.19 label Apr 12, 2023
@silverwind
Copy link
Member Author

No backport as it won't be a clean merge and the issues are rather minor.

@silverwind
Copy link
Member Author

silverwind commented Apr 12, 2023 •
edited
Loading

Small style tweak done to rendering error blocks: Font size now matches the code block below, border is removed:

Screenshot 2023-04-12 at 15 06 59

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Apr 15, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Apr 17, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 17, 2023
@techknowlogick
Copy link
Member

🤖 🎺

@silverwind silverwind merged commit dcde470 into go-gitea:main Apr 17, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Apr 17, 2023
@silverwind silverwind deleted the double-err branch April 17, 2023 10:10
zjjhot added a commit to zjjhot/gitea that referenced this pull request Apr 18, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
bab3026 
* giteaofficial/main: (25 commits)
  zh-cn support on doc pages (go-gitea#24166)
  [skip ci] Updated translations via Crowdin
  Use double quotes consistently in en-US (go-gitea#24141)
  Use correct locale key for forks page (go-gitea#24172)
  Improve Wiki TOC (go-gitea#24137)
  Localize activity heatmap (except tooltip) (go-gitea#24131)
  Support triggering workflows by wiki related events (go-gitea#24119)
  add CLI command to register runner tokens (go-gitea#23762)
  Add new user types `reserved`, `bot`, and `remote` (go-gitea#24026)
  Fix Org edit page bugs: renaming detection, maxlength (go-gitea#24161)
  Make HAS_GO a simply expanded variable (go-gitea#24169)
  Support converting varchar to nvarchar for mssql database (go-gitea#24105)
  Fix math and mermaid rendering bugs (go-gitea#24049)
  Refactor locale number (go-gitea#24134)
  [skip ci] Updated translations via Crowdin
  Use 1.18's aria role for dropdown menus (go-gitea#24144)
  Set EasyMDE heading font-size to the same size as the resulting markdown (go-gitea#24151)
  Fix 2-dot direct compare to use the right base commit (go-gitea#24133)
  Add migration to fix external unit access mode of owner/admin team (go-gitea#24117)
  Remove untranslatable `on_date` key (go-gitea#24106)
  ...
@silverwind silverwind mentioned this pull request May 11, 2023
Closed
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jul 31, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@HesterG HesterG HesterG approved these changes

@lunny lunny lunny approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. outdated/theme/markdown type/bug
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.
5 participants
@silverwind @techknowlogick @lunny @HesterG @GiteaBot