Refactor markdown properties to handle whitespace#559
Refactor markdown properties to handle whitespace#559Eulenhorn wants to merge 1 commit intoPhalcode:developfrom
Conversation
Updated the set accessors for DescriptionMarkdown and NotesMarkdown properties to replace tabs and spaces with non-breaking spaces.
|
There was a problem hiding this comment.
Pull request overview
This PR addresses a rendering bug where markdown text was displaying with a white background, making it unreadable. The fix replaces tabs and four consecutive spaces with non-breaking spaces in markdown properties to prevent the rendering issue.
- Modified the
DescriptionMarkdownandNotesMarkdownproperty setters to transform whitespace characters - Replaced tabs and four-space sequences with four non-breaking space characters (\u00A0)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| descriptionMarkdown = value? | ||
| .Replace("\t", "\u00A0\u00A0\u00A0\u00A0") | ||
| .Replace(" ", "\u00A0\u00A0\u00A0\u00A0"); |
There was a problem hiding this comment.
The replacement values "\u00A0\u00A0\u00A0\u00A0" are magic strings that appear multiple times without explanation. The decision to replace tabs and four spaces with exactly four non-breaking spaces is not documented. Consider extracting these as named constants with clear documentation explaining why this specific replacement is necessary for fixing the white background issue.
| descriptionMarkdown = value? | ||
| .Replace("\t", "\u00A0\u00A0\u00A0\u00A0") | ||
| .Replace(" ", "\u00A0\u00A0\u00A0\u00A0"); |
There was a problem hiding this comment.
The string.Replace method operates sequentially, which can lead to unintended behavior. When " " (four spaces) is replaced with four non-breaking spaces, if the original text contains sequences of more than four spaces, each group of four consecutive spaces will be replaced independently. For example, eight consecutive spaces would become eight non-breaking spaces (correct), but six consecutive spaces would become four non-breaking spaces followed by two regular spaces (inconsistent). Consider replacing multiple spaces in a single operation or using a more comprehensive approach to handle all space sequences consistently.
| notesMarkdown = value? | ||
| .Replace("\t", "\u00A0\u00A0\u00A0\u00A0") | ||
| .Replace(" ", "\u00A0\u00A0\u00A0\u00A0"); |
There was a problem hiding this comment.
The whitespace replacement logic is duplicated in both DescriptionMarkdown and NotesMarkdown setters. This violates the DRY principle and makes maintenance more difficult. If the replacement logic needs to be changed in the future, it will need to be updated in both places. Consider extracting this logic into a private helper method.
|
Thank you for the PR, @Yelo420 is back from holidays today and will take a look at it. For the future it would cool if you created a bug-issue first, then started working on the PR. We have no idea what bug you are talking about. |
|
This is addressing the issue where the Markdown interpreter treats four spaces as a code block. This causes the text to render with a white background and strips all formatting, making the white text unreadable. This is replacing standard spaces with non-breaking spaces because code blocks are unnecessary in a game description and they break the visual presentation. |
i can not reproduce the issue. would you mind adding a screenshot that shows the bug and showing us the exact markdown you entered to trigger the bug? |
2 participants
Updated the set accessors for DescriptionMarkdown and NotesMarkdown properties to replace tabs and spaces with non-breaking spaces. This fixes a bug making Text appear with a white background and being unreadable.