Skip to content

Fix EmphasisAPI Leaking Layers, Fix Emphasis Drawing #79

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

Merged
merged 4 commits into from
Apr 7, 2025

Conversation

thecoolwinter
Copy link
Contributor

Description

  • Fixes an emphasis manager bug where it could leak layers without removing their CALayers when replacing flash emphases with other emphases.
  • Updates the emphasis drawing code to correctly draw underlines.
  • Updates the roundedPathForRange to correctly handle emphases that span multiple line fragments.

During the last point, I discovered that the emphasis manager doesn't correctly draw the text on emphases that span multiple line fragments. This makes sense, as right now it's just using a CATextLayer that has no knowledge of line breaks.

In the future, that text layer should be replaced by a custom layer that draws the text using knowledge of existing line fragments positions. However, I think the issues this PR fixes are too important to wait until that can be done.

Also note that that change will be critical to drag and drop, so it will absolutely be done and I have a branch somewhere with some of that work already completed.

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

Screen recording showing the text layer bug mentioned above, as well as the fixed multi-fragment drawing, and the fixed leak.

Prior to this, when switching from flash to any other emphasis type, there was a good chance one of the new emphases would be kept. This was due to the emphasis manager assuming that the emphasis being flashed was kept around after the animation delay. This has been fixed in this PR.

Screen.Recording.2025-04-07.at.12.11.10.PM.mov

@thecoolwinter thecoolwinter marked this pull request as draft April 7, 2025 17:31
@thecoolwinter thecoolwinter marked this pull request as ready for review April 7, 2025 17:45
@thecoolwinter thecoolwinter merged commit 47faec9 into CodeEditApp:main Apr 7, 2025
2 checks passed
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.

1 participant