On Fri, 15 Nov 2019 12:59:15 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
> On Wed, 13 Nov 2019 11:26:54 GMT, Hadzic Samir <shad...@openjdk.org> wrote: > >> On Thu, 7 Nov 2019 12:07:55 GMT, Jeanette Winzenburg <faste...@openjdk.org> >> wrote: >> >>> On Fri, 1 Nov 2019 10:59:57 GMT, Hadzic Samir <shad...@openjdk.org> wrote: >>> >>>> On Tue, 29 Oct 2019 13:19:27 GMT, Hadzic Samir <shad...@openjdk.org> wrote: >>>> >>>>> On Wed, 9 Oct 2019 16:01:38 GMT, Kevin Rushforth <k...@openjdk.org> wrote: >>>>> >>>>>> On Wed, 9 Oct 2019 12:26:31 GMT, Hadzic Samir <shad...@openjdk.org> >>>>>> wrote: >>>>>> >>>>>>> On Mon, 7 Oct 2019 10:22:11 GMT, Jeanette Winzenburg >>>>>>> <faste...@openjdk.org> wrote: >>>>>>> >>>>>>>> On Fri, 4 Oct 2019 06:13:48 GMT, Hadzic Samir <shad...@openjdk.org> >>>>>>>> wrote: >>>>>>>> >>>>>>>>> Allright, this is a fix for JDK-8207957 >>>>>>>>> >>>>>>>>> ---------------- >>>>>>>>> >>>>>>>>> Commits: >>>>>>>>> - 969ebb51: Fixing TableColumnHeaderTest >>>>>>>>> - 9d379619: Removing Tablecolumnbasehelper >>>>>>>>> - 4fe020fc: Fix javadoc for TableColumnHeader >>>>>>>>> - c422c80f: Minor modification and uni test added. >>>>>>>>> - b2bdfb5b: Change resizeColumn to protected without static in order >>>>>>>>> to be able to override >>>>>>>>> - 3b9b7903: Second option for resizeColumnToFitContent in >>>>>>>>> TableColumnHeader >>>>>>>>> - d91c56e5: First attempt to extract resizeColumnToFitContent >>>>>>>>> >>>>>>>>> Changes: https://git.openjdk.java.net/jfx/pull/6/files >>>>>>>>> Webrev: https://webrevs.openjdk.java.net/jfx/6/webrev.00 >>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8207957 >>>>>>>>> Stats: 523 lines in 4 files changed: 330 ins; 187 del; 6 mod >>>>>>>>> Patch: https://git.openjdk.java.net/jfx/pull/6.diff >>>>>>>>> Fetch: git fetch https://git.openjdk.java.net/jfx pull/6/head:pull/6 >>>>>>>> >>>>>>>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java >>>>>>>> line 600: >>>>>>>> >>>>>>>>> 599: * expensive if the number of rows is large. Subclass can >>>>>>>>> either call this method or override it (no need to call >>>>>>>>> 600: * {@code super()}) to provide their custom algorithm. >>>>>>>>> 601: * >>>>>>>> >>>>>>>> see >>>>>>>> https://github.com/javafxports/openjdk-jfx/pull/289#discussion_r245482213 >>>>>>>> - I think I made a suggestion (that probably needs some native >>>>>>>> speaker's fine tuning :) >>>>>>> >>>>>>> Allright @kleopatra , I have indeed removed the TableColumn argument. >>>>>>> It is clearer now that we are resizing the TableColumn of the header. >>>>>>> >>>>>>> I have updated the description a bit but really I'm really not good at >>>>>>> method description so I'm open to all suggestions.. >>>>>> >>>>>> A Draft CSR was filed here: >>>>>> [JDK-8215554](https://bugs.openjdk.java.net/browse/JDK-8215554). It will >>>>>> need to be updated once the API review is complete. >>>>> >>>>> Do you think this looks good now @kleopatra @nlisker ? >>>> >>>> Yes I was waiting for the final 'go'. I will update it asap, thanks >>>> >>>> Le ven. 1 nov. 2019 à 09:49, Jeanette Winzenburg <notificati...@github.com> >>>> a écrit : >>>> >>>>> *@kleopatra* commented on this pull request. >>>>> ------------------------------ >>>>> >>>>> In >>>>> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableColumnHeader.java >>>>> <https://github.com/openjdk/jfx/pull/6#discussion_r341492115>: >>>>> >>>>> > + * Resizes this {@code TableColumnHeader}'s column to fit the >>>>> > width of its content. >>>>> + * >>>>> + * @implSpec The resulting column width for this implementation is >>>>> the maximum of the preferred width of the header >>>>> + * cell and the preferred width of the first {@code maxRow} cells. >>>>> + * <p> >>>>> + * Subclasses can either use this method or override it (without the >>>>> need to call {@code super()}) to provide their >>>>> + * custom implementation (such as ones that exclude the header, >>>>> exclude {@code null} content, compute the minimum >>>>> + * width etc.). >>>>> + * >>>>> + * @param maxRows the number of rows considered when resizing. If -1 >>>>> is given, all rows are considered. >>>>> + * @since 14 >>>>> >>>>> looks good to me :) You would have to update the crs eventually, right? >>>>> >>>>> — >>>>> You are receiving this because you authored the thread. >>>>> Reply to this email directly, view it on GitHub >>>>> <https://github.com/openjdk/jfx/pull/6?email_source=notifications&email_token=ACEL43PISZ6DM46QLSYDLR3QRPURHA5CNFSM4I5EYYAKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCJ7XYEQ#discussion_r341492115>, >>>>> or unsubscribe >>>>> <https://github.com/notifications/unsubscribe-auth/ACEL43PROHPIZYN7WA5M6KDQRPURHANCNFSM4I5EYYAA> >>>>> . >>>>> >>> >>> As to the test, it's a bit ... confusing, will try to suggest some changes >>> - take them with a grain of salt, though, because I'm still groping to find >>> my path through the wilderness here :) >> >> I have changed the unit test according to @kleopatra review and also add two >> more unit test regarding the headerContent and maxRows. > > looks good to me - would approve if I could :) > looks good to me - would approve if I could :) You can add yourself as a reviewer, even though it wouldn't count as an approving review by someone with a "R"eviewer role in the project. PR: https://git.openjdk.java.net/jfx/pull/6
