-
Notifications
You must be signed in to change notification settings - Fork 52
Use DrawableTextUtilities for Label and TextFlow figures #817
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
| /** | ||
| * @since 3.21 | ||
| */ | ||
| public static TextUtilities getTextUtilities(IFigure figure) { | ||
| if (getRoot(figure) instanceof LightweightSystem.RootFigure rootFigure) { | ||
| return rootFigure.getTextUtilities(); | ||
| } | ||
| return TextUtilities.INSTANCE; | ||
| } | ||
|
|
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.
I'm still not sure where to put this. In principle, it might also be directly added to the IFigure?
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.
I think I figure sounds like a very good place for this. Should we also move the getRoot there? Or better add?
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.
Should we also move the getRoot there? Or better add?
I think that makes sense. The way it is currently done is by using something like, for e.g. the font and colors:
public Object xyz() {
if (getParent() != null) {
return getParent().xyz();
}
return null;
}So that I'd do as a separate PR, together with cleaning up all of the other methods.
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.
In hindsight, I don't think the getRoot() method is a good idea. In principle, any parent can override the getTextUtilities() method and provide its own implementation. This is how it's done for all other methods that are overridden by the root figure.
Changing this here feels odd. There are also several figures that access TextUtilities.INSTANCE or the static FigureUtilities methods directly. This I'd address with a separate PR.
a750f57 to
5d4b806
Compare
| return TextUtilities.INSTANCE; | ||
| } | ||
|
|
||
| } |
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.
Weirdly enough, increasing the visibility doesn't break binary compatibility, but may cause source-incompatibilities if overridden by subclasses.
This adds a new getTextUtilities() method to the IFigure interface. The method is overriden by the RootFigure and returns an object of type DrawableTextUtilities, which is created for the canvas hooked to the LWS. When using this method, users are guaranteed that the font sizes returned calculated by this instance match the zoom level of the monitor they are painted on.
5d4b806 to
7725641
Compare
|
I also just noticed that the fix for #816 is not applied to the |
This adds a new getTextUtilities() method to the IFigure interface. The method is overriden by the RootFigure and returns an object of type DrawableTextUtilities, which is created for the canvas hooked to the LWS.
When using this method, users are guaranteed that the font sizes returned calculated by this instance match the zoom level of the monitor they are painted on.