-
Notifications
You must be signed in to change notification settings - Fork 28
Added tests for multiline text support #261
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
base: master
Are you sure you want to change the base?
Conversation
Tests check that \n converts to <br/> in: - plot titles (ggtitle) - axis titles (xlab/ylab) - legend titles - geom_text labels Currently fail since feature not implemented. Addresses #221
Changes: - R/z_multiline.R: new helper to convert \n to <br/> - R/z_animint.R: apply conversion to plot title, x/y axis titles - R/z_animintHelpers.R: apply conversion to legend titles and geom_text labels - inst/htmljs/animint.js: render <br/> as SVG tspan elements, fix overlap with proper height calculation - DESCRIPTION: add z_multiline.R to Collate - NEWS.md: document multiline text support Fixes #221
Add Multi-line Text Support Throughout animint2Fixes #221 SummaryThis PR enables newline characters ( What Now Works✅ Plot titles - Visual VerificationThe screenshot below demonstrates multi-line text working correctly in all contexts:
Example Usagelibrary(animint2)
data <- data.frame(
x = 1:5,
y = 1:5,
group = c("A", "B", "C", "A", "B"),
label = c("Single", "Two\nLines", "Three\nLines\nHere", "Text", "Label")
)
viz <- animint(
plot1 = ggplot(data, aes(x, y, color = group, label = label)) +
geom_point(size = 5) +
geom_text(vjust = -1) +
ggtitle("Multi-line Test\nIssue #221 Demonstration") +
xlab("X Axis\n(Horizontal Position)") +
ylab("Y Axis\n(Vertical Position)") +
scale_color_discrete(name = "Data Group\n(Click to Select)")
)
animint2dir(viz, "output-dir") |
|
No obvious timing issues in HEAD=multiline-text-support-221 Generated via commit acbc6da Download link for the artifact containing the test results: ↓ atime-results.zip
|
|
Ran tests locally - all 19 multiline text tests pass successfully. The test failures shown are due to missing optional packages (mapproj, sp, XML) and GitHub API permissions, not related to this PR. Sir can you please help me figure out on how to pass the JS_Coverage test suite . |
|
To get this PR to close the corresponding issue, please add Closes #221 on its own line, in the first comment of the PR. |
|
looks like a step in the right direction, thanks! |
|
Also in #261 (comment) it seems like there is an inconsistency between the X and Y axes.
can this be adjusted to be more consistent please? |
tdhock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add tests then fix
Tests check: - plot title doesn't overlap with plot area - x-axis and y-axis spacing is consistent - multiline spacing matches single-line baseline Tests currently fail, demonstrating the bugs in current code. Addresses feedback on PR #261.
Two fixes: 1. Add 5px margin below title to prevent overlap with plot area 2. Use same base spacing (30px) for both X and Y axis labels Both axes now have consistent spacing, and multiline titles stay above the plot area. Addresses feedback on PR #261.
tdhock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please simplify height/width calculation
- removed hardcoded -1000 positioning - reuse setMultilineText() instead of duplicating logic - rely on getBBox() for accurate measurements fixes spacing issues
…nt/animint2 into multiline-text-support-221
…nt/animint2 into multiline-text-support-221
|
can you please post an updated screen shot? |
Axis titles were stuck at 16px even when theme(text=element_text(size=30)) was set. Now they properly inherit the size from theme settings. I saw your #64 coment
|
Sir @tdhock Fixed the axis title font size issue. Axis titles now scale correctly with Changes: Tested with different font sizes (10, 30, 50) - all working locally on my system . Please Sir review it and give your feedback !!! |
Add tests to verify axis titles correctly inherit theme text size. This improves code coverage for the axis title font-size fix. Tests verify: - Axis titles scale with theme(text=element_text(size=30)) - Default size is used when theme not specified - Both xtitle_size and ytitle_size are correctly extracted
Document multiline text spacing fixes and axis title font-size scaling fix in NEWS.md
|
Sir @tdhock Please give your feedback !!! |
tdhock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please revise and explain
- Use 2-space indentation consistently - Remove extra blank lines in functions - Document 1.2 lineHeight magic number - Keep .each() for D3 element selection
|
Sir @tdhock I have fixed all code style issues :=>
All tests passed locally on my system . |
|
to understand why the test cases fail, you should try running them line by line under chromote, after tests_init(), and then look at the output in the remote controlled web browser window https://github.com/animint/animint2/wiki/Testing |
The xtitle text element was missing the transform attribute, which caused renderer tests to fail with 'subscript out of bounds' errors when trying to access xtitle.attrs[["transform"]]. Added transform='translate(0,0)' to match ytitle and plottitle elements, ensuring consistent structure for all title elements. This fixes test failures in: - test-renderer1-facet-space.R (line 42) - test-renderer1-facet-trivial.R (line 32) Addresses maintainer feedback from PR #261.
Tests expect transform='translate(x,y)' pattern but we were using separate x/y attributes plus 'translate(0,0)'. Fixed all title elements to use transform for positioning: - plottitle: translate(plotdim.title.x, plotdim.title.y) - xtitle: translate((xtitle_left+xtitle_right)/2, xtitle_y) - ytitle: translate(ytitle_x, (ytitle_top+ytitle_bottom)/2)rotate(270) This matches the original code structure and the translatePattern regex expected by tests. Fixes test-renderer1-facet-space.R and test-renderer1-facet-trivial.R
Changed all jsonlite::fromJSON() calls to RJSONIO::fromJSON(). Removed insufficient axis title size tests. Added renderer test verifying multiline text with large font does not overlap plot area (as requested).
|
please click Resolve conversation so reviewers can see which things you have already fixed. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #261 +/- ##
==========================================
+ Coverage 72.77% 72.97% +0.20%
==========================================
Files 164 165 +1
Lines 8805 8871 +66
==========================================
+ Hits 6408 6474 +66
Misses 2397 2397
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
please post updated screenshot |
|
this looks better. |




Add Multi-line Text Support Throughout animint2
Fixes #221
This PR enables newline characters (\n) to work in all text elements of animint2 visualizations. Users can now create multi-line text in plot titles, axis labels, legend titles, and geom_text labels.
What Now Works:
✅ Plot titles - ggtitle("Line 1\nLine 2")
✅ X-axis titles - xlab("X Axis\nSubtitle")
✅ Y-axis titles - ylab("Y Axis\nSubtitle")
✅ Legend titles - scale_color_discrete(name="Legend\nTitle")
✅ Text labels - geom_text(aes(label="Text\nLabel"))
✅ Tooltips - Already worked, still works (backward compatible)
Below is the screenshots of the scripts i ran locally to verify the test locally they fail as aspected

