On Tue, 10 Dec 2019 18:54:08 GMT, Kevin Rushforth <k...@openjdk.org> wrote:
>> Overall this looks good to me with one "must fix" API issue and one >> additional minor comment. >> >> In addition to the automated unit test, it might be nice to have a simple >> app (in `apps/toys`) with a slider to control the tab size of a Text and/or >> TextFlow. This could be done as a follow-on issue if you prefer. >> >> Once you fix the public API issue, you can add ithe API to the CSR and I'll >> review it. The javadoc, public methods, and CSS additions for the two >> classes should go into the CSR in the specification section. See >> [JDK-8195139](https://bugs.openjdk.java.net/browse/JDK-8195139) for a good >> example. >> >> @prrace still needs to review the implementation and API as well. > >> Should this PR also add a tabSize property to controls such as TextArea? Or >> should that be a different PR after this one is merged? > > This would need to be a new enhancement and would first need to be discussed > on the openjfx-dev mailing list. There are additional things to consider in > order to support tab size for controls, and it isn't clear whether there is > enough benefit in doing it. As a follow-on point to the missing public method in TextFlow, can you add unit tests for the API methods on both `Text` and `TextFlow`? A good way to do that is to have a test for all combinations of setting the value via the setter and the property method, and getting the value via the getter and the property method. ------------- PR: https://git.openjdk.java.net/jfx/pull/32
