Reviewed-by: kcr, mstrauss
-
Commit messages:
- 8276553: ListView scrollTo() is broken after fix for JDK-8089589
Changes: https://git.openjdk.java.net/jfx17u/pull/50/files
Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u=50=00
Issue:
On Fri, 3 Dec 2021 20:46:46 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
On Fri, 3 Dec 2021 20:46:46 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
> After (re)setting the number of elements, make sure to do at least some
> estimation of the total size.
> Added a testcase for this scenario.
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
add checks on positive values on counters
On Fri, 3 Dec 2021 18:47:24 GMT, Kevin Rushforth wrote:
> The fix and new test looks fine. I added one comment/question regarding the
> existing tests that check a count, but I'm OK with either answer.
The suggestion to test for positive values of the counters is a good idea. The
lower this
On Fri, 3 Dec 2021 11:25:43 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
On Fri, 3 Dec 2021 18:55:29 GMT, Michael Strauß wrote:
> Would it be a good idea to fix the implementation of the `position` property
> as part of this PR (it has the same problem as `cellCount` before the fix)?
> If not, a follow-up ticket should be filed.
A follow-up issue,
On Fri, 3 Dec 2021 11:25:43 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
On Fri, 3 Dec 2021 15:10:35 GMT, Johan Vos wrote:
>>> The hard values have been changed a number of times, and I believe it is
>>> not really a good metric.
>>
>> I agree completely.
>>
>>> Rather than requiring that the amount of calls should be a fixed number, I
>>> think it makes more
On Fri, 3 Dec 2021 11:25:43 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
On Fri, 3 Dec 2021 14:41:11 GMT, Ajit Ghaisas wrote:
>> The hard values have been changed a number of times, and I believe it is not
>> really a good metric.
>> What we want to ensure is that there is no functional regression and no
>> performance penalty. The tests calculate the number of
On Fri, 3 Dec 2021 11:16:28 GMT, Johan Vos wrote:
> The hard values have been changed a number of times, and I believe it is not
> really a good metric.
I agree completely.
> Rather than requiring that the amount of calls should be a fixed number, I
> think it makes more sense to ensure that
On Fri, 3 Dec 2021 10:24:52 GMT, Ajit Ghaisas wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move functionality in the setCellCount() to the invalidated block.
>> Some hard numbers used in tests (counters for
> After (re)setting the number of elements, make sure to do at least some
> estimation of the total size.
> Added a testcase for this scenario.
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
process reviewer comment (simplify call)
On Fri, 3 Dec 2021 10:20:43 GMT, Ajit Ghaisas wrote:
>> Johan Vos has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Move functionality in the setCellCount() to the invalidated block.
>> Some hard numbers used in tests (counters for
On Tue, 30 Nov 2021 11:02:41 GMT, Johan Vos wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> Johan Vos has updated the pull request incrementally with one additional
> commit since the
> After (re)setting the number of elements, make sure to do at least some
> estimation of the total size.
> Added a testcase for this scenario.
Johan Vos has updated the pull request incrementally with one additional commit
since the last revision:
Move functionality in the setCellCount() to
On Mon, 29 Nov 2021 17:00:10 GMT, Kevin Rushforth wrote:
>> Exactly. This ensures consistent behavior regardless of how a property value
>> is set.
>
> I should add that this looks like a preexisting problem, but one that would
> be good to fix if possible.
Thanks for catching. I moved the
On Mon, 29 Nov 2021 16:59:02 GMT, Kevin Rushforth wrote:
>> good catch! the "additional" stuff typically is done in the properties'
>> invalidated ..
>
> Exactly. This ensures consistent behavior regardless of how a property value
> is set.
I should add that this looks like a preexisting
On Mon, 29 Nov 2021 16:55:16 GMT, Jeanette Winzenburg
wrote:
>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
>> line 903:
>>
>>> 901: recalculateAndImproveEstimatedSize(2);
>>> 902:
>>> 903: cellCount.set(value);
>>
>> Can this be
On Mon, 29 Nov 2021 14:38:10 GMT, Michael Strauß wrote:
>> After (re)setting the number of elements, make sure to do at least some
>> estimation of the total size.
>> Added a testcase for this scenario.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> line
On Mon, 29 Nov 2021 11:56:45 GMT, Johan Vos wrote:
> After (re)setting the number of elements, make sure to do at least some
> estimation of the total size.
> Added a testcase for this scenario.
modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
line 903:
> 901:
After (re)setting the number of elements, make sure to do at least some
estimation of the total size.
Added a testcase for this scenario.
-
Commit messages:
- After (re)setting the number of elements, make sure to do at least some
estimation of the total size.
Changes:
23 matches
Mail list logo
