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 spacing for callout titles #587

Merged
merged 3 commits into from
May 28, 2024
Merged

Conversation

froggleston
Copy link
Contributor

@froggleston froggleston commented May 22, 2024

This PR is a work in progress (as in it hasn't been tested anywhere near enough) to fix #562 with spacing in callout title headers. Currently, any HTML tags within the h3 title will lose any spacing due to the removal of whitespace when checking for translations.

The fix moves the trimming to inside the translation application function (apply_transformations) so leaves the tag spacing unchanged when replacing any translated elements. However, this hasn't been thoroughly checked.

@froggleston
Copy link
Contributor Author

The fix is more complicated it seems as translation messes with what strings are sanitised and matched:

  • Any string prior to translation is stripped of whitespace, to ensure exact matching.
  • However, h3 titles with <code> blocks have trailing/leading whitespace also removed, leading to the issue described.

I've addressed this by moving the check and removal of whitespace to inside the apply_translation function, allowing us to retain knowledge of if the string did indeed have whitespace and to put it back if removed.

Similarly, I have excluded h3 titles for the translation check (and whitespace handling) which have inner tags, like <code>. Whilst this isn't optimal, I'm unsure of how to improve it without using some sophisticated matching for translation strings.

@froggleston froggleston changed the title [WIP] Fix callout spacing for #562 Fix callout spacing for #562 May 23, 2024
@froggleston froggleston changed the title Fix callout spacing for #562 Fix spacing for callout titles May 23, 2024
@ErinBecker
Copy link
Contributor

I can confirm that this works for English-language lessons (tested with Foundations of Astronomical Data Science and Collaborative Lesson Development Training (screenshots below))

Screenshot 2024-05-24 at 10 43 51 AM Screenshot 2024-05-24 at 10 45 22 AM Screenshot 2024-05-24 at 10 45 05 AM

Agreed with @froggleston that this should also be tested on lessons that go through the translation process.

Copy link
Member

@tobyhodges tobyhodges left a comment

Choose a reason for hiding this comment

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

I have tested this on a local version of a lesson in english and the version of git-novice localised to Japanese. It looks good in both cases (screengrab from the Japanese lesson build below) so I think this is ready to merge.

Screenshot of episode 4, Tracking Changes, of the Software Carpentry Git lesson translated to Japanese, showing correct spacing between the word bio formatted as inline code in a callout title, followed by the rest of the title in standard text formatting

@froggleston froggleston merged commit a215171 into main May 28, 2024
12 checks passed
@froggleston froggleston deleted the callout_title_code_spacing branch May 28, 2024 15:04
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.

Callout title does not correctly render when code formatting is included
3 participants