On Wed, 9 Oct 2024 05:44:00 GMT, Phil Race <p...@openjdk.org> wrote:
>> Andy Goryachev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comments
>
> modules/javafx.graphics/src/main/java/javafx/scene/text/LayoutInfo.java line
> 34:
>
>> 32: * @since 24
>> 33: */
>> 34: public interface LayoutInfo {
>
> Why an interface ? It is easier to evolve a class when you want to add more
> info.
> Also it should be sealed unless you can show why it is important that
> applications can create instances.
>
> It is being exposed for the benefit of RT but the class doc itself doesn't
> say when you might want to get one and why.
> It might also benefit from a succinct description of what "the text layout"
> means and encompasses.
>
> Oh and how do you know when it is no longer valid ?
good points, thanks! please let me know if the update addressed your concerns.
a couple of notes:
- converted to an abstract class. I feel weird referring to an internal
implementation at the API level (`public sealed abstract class LayoutInfo
permits com.sun.javafx.text.PrismLayoutInfo`, yikes!)
- why we get it: we don't explain why we need HitInfo or caret shapes or range
shapes, do we?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1596#discussion_r1794095627
