RFR: 8251480: TableColumnHeader: calc of cell width must respect row styling [v2]

Robert Lichtenberger rlichten at openjdk.java.net
Wed Mar 30 11:02:44 UTC 2022


On Tue, 29 Mar 2022 20:35:10 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Robert Lichtenberger has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   8251480: TableColumnHeader: calc of cell width must respect row styling
>>   
>>   Old behaviour restored for table rows with custom skins.
>>   Unnecessary call to cell.applyCss() removed.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java line 715:
> 
>> 713:             tableRow = createMeasureRow(tv, tableSkin, null);
>> 714:         }
>> 715:         assert tableRow.getSkin() instanceof SkinBase<?>;
> 
> Not sure if there is some convention about code asserts, maybe some else can tell. At least I never saw one in a PR here.

I found assertions in various other parts of the code (e.g. `javafx.scene.control.skin.VirtualFlow<T>`) so I assumed this is ok.

> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java line 276:
> 
>> 274:     }
>> 275: 
>> 276:     private TableRow<Person> createSmallRow(TableView<Person> tableView) {
> 
> Minor: Move this method to the other row method below?

You're right. I'll move the method.

> modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableColumnHeaderTest.java line 295:
> 
>> 293:     private TableRow<Person> createCustomRow(TableView<Person> tableView) {
>> 294:         TableRow<Person> row = new TableRow<>() {
>> 295:             protected javafx.scene.control.Skin<?> createDefaultSkin() {
> 
> Very minor: `javafx.scene.control.` is not needed

You're right. I'll remove this.

-------------

PR: https://git.openjdk.java.net/jfx/pull/757


More information about the openjfx-dev mailing list