On Mon, 29 Aug 2022 22:40:15 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> The corner region is currently laid out with this assumption: >> >> // position the top-right rectangle (which sits above the scrollbar) >> >> However, the vertical scrollbar is not always visible. Therefore, when that >> is the case, the corner region is laid out on top of the table column >> headers, overlapping any of their graphic or text content. In case of the >> last visible column header, it is not always possible to scroll horizontally >> to see the full content, either because of a constrained resize policy or >> because there is no more space left after the last column. >> >> This PR fixes this issue by considering as padding for the last visible >> column header the presence of the corner region over it, only when the >> vertical scrollbar is not visible and the corner region is. >> >> A couple of tests have been added to both TableViewTest and >> TreeTableViewTest. >> >> Test 1: Before and after the changes : >> >> <img width="312" alt="image" >> src="https://user-images.githubusercontent.com/2043230/187190928-efc29e38-8de2-4daf-8eff-a218d6fae181.png"> >> <img width="312" alt="image" >> src="https://user-images.githubusercontent.com/2043230/187190991-deaa88b3-2f52-48bc-b3ec-107a5151f7b5.png"> >> >> Test 2: Before (sort arrow is there, but not visible) and after the changes: >> <img width="412" alt="image" >> src="https://user-images.githubusercontent.com/2043230/187191310-fb348d8c-84fc-4254-a1a5-91811bf09b79.png"> >> <img width="412" alt="image" >> src="https://user-images.githubusercontent.com/2043230/187191358-0d4937cb-b3b4-4530-9489-ff8cbf0c8443.png"> > > modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewTest.java > line 5883: > >> 5881: assertTrue(thumbMaxX < cornerMinX); >> 5882: >> 5883: sl.dispose(); > > may be in try/finally to dispose of the StageLoader if test throws an > exception? There are no exceptions expected to happen in these new tests and I don't see that we use try-finally with `StageLoader` by default, is this change really required? ------------- PR: https://git.openjdk.org/jfx/pull/886