-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
scroll text if width of Textbox is exceeded #4293
Conversation
f00a8b5
to
ce7b505
Compare
not sure how to add a test. happy to do so if someone suggests a way |
again, could i please get a review? here's demo code and a movie of the amazing new functionality this PR offers to Makie :)
Screen.Recording.2024-09-16.at.9.49.53.AM.mov |
ce7b505
to
02257fa
Compare
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.
This should have a visual reference test I think. Textbox doesn't have its own one so a new one can be added in figures_and_makielayout.jl. All the branches that currently codecov complains about should be handled there, like one normal textbox, one with text scrolled.
With regards to the implementation, I guess this all could be handled a bit more cleanly by a translate!
on the text and the cursor plot objects instead of modifying their input args. That would correspond more to the idea of "scrolling", too. I don't really like textplot.converted[1][] = [Point3f(oldv...
as at some point it gets hard to reason about what influences what. Textbox is already not so easy to understand.
b952148
to
39bea74
Compare
thanks for the suggestion regarding the implementation! i have refactored to use |
b726427
to
423a1c9
Compare
i have added visual reference tests, but am having trouble uploading them. when i click on "Update reference images with selection", a popup appears with a "v0.21.0" tag. irrespective of whether i stick with that or choose another (e.g. "v0.22.0", or "master") i get an error on the julia REPL. is this because i don't have write permissions to the required repo? |
Yes you need commit rights to use the reference updater. Contributors can only add new test code which notifies committers in the PR status that new images are needed |
thanks @jkrumbiegel for the info. let me finish the tests and i'll ping you for another review. |
7193ee2
to
896f515
Compare
ready for review again. codecov must not consider the reference tests as those three lines are definitely used there |
896f515
to
63c450e
Compare
63c450e
to
c8c76e0
Compare
the ReferenceTests report that "Violin plots differently scaled.png" is different. how could that possibly be? |
That's not a problem of the pr ;) we've been updating the reference images and there's likely a Missmatch |
Thanks :) |
Description
Fixes #4268
Type of change
Checklist