fix: Preserve line breaks in info panel text item#3379
Conversation
|
I will reformat the title to use the proper commit message syntax. |
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning and design rather than using review cycles to fix low-quality code. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughA single-line CSS change was applied to the TextElement component's paragraph rendering, adding an inline ChangesText formatting fix
Estimated code review effort: 1 (Trivial) | ~2 minutes Related issues: Fixes line breaks being lost when displaying multi-line text in the info panel ( Suggested labels: bug, ready-to-merge Suggested reviewers: mtrezza 🐰 A rabbit hops through lines of code, 🚥 Pre-merge checks | ✅ 5 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
mtrezza
left a comment
There was a problem hiding this comment.
No images, CR hit limit, branch not up-to-date. req again when all done
|
@mtrezza images restored |
|
What's the source of text that now shows line breaks? What's causing these breaks? |
|
This here is the response of a cloud function, if you returned |
|
Condensing text in a side panel is a feature I'd argue, not a bug. There is limited space in a side panel. It currently condenses line breaks, tabs, spaces. Respecting just line breaks seems arbitrary to me. And what if someone requests to respect tabs and spaces tomorrow? I believe by default it should collapse (status quo) because information density is key. Formatting should be deterministic in code - it's HTML after all. I'd rather lean towards allowing HTML tags, which gives a dev more freedom, including Btw, doesn't the text element already support a |
|
Respecting newlines isn't really arbitrary - That said, you're right that a dev can already pass { type: 'text', text: '...', style: { whiteSpace: 'pre-line' } }i think it'd stump most dashboard devs to have to reach for CSS just to keep a line break they explicitly typed. On raw HTML, IMV i'd steer clear. The text often interpolates user data ( Don't feel strongly though - happy for you to close this and the issue if you're good with the workaround. |
I don't understand the argument - tabs are also a unicode char. It is arbitrary in the sense that the default is to condense text. Why do you want to single out line breaks but keep condensing tabs and spaces?
I would be very surprised - that should never be an expectation in HTML. Not only is it not the default behavior, condensing text is also the historic normal, long before CSS. |
Closes #2651
Info panel
textitems lost line breaks: newlines (\n) in the value collapsed into spaces because the text was rendered in a plain<p>(defaultwhite-space: normal). Settingwhite-space: pre-linepreserves author-entered line breaks while still collapsing redundant spaces. The text remains an escaped React child (nodangerouslySetInnerHTML), so there is no XSS risk.Before
After
Summary by CodeRabbit