Refactor markdown properties to handle whitespace #559
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.
1 participant
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.