On Thu, 12 Mar 2020 12:42:28 GMT, Jeanette Winzenburg <faste...@openjdk.org> wrote:
>> Ajit Ghaisas has updated the pull request incrementally with one additional >> commit since the last revision: >> >> extra space removed > > sry for the spelling errors you inherited :) @aghaisas can't see your last response here (looks like I'm fighting the system here - and loosing again ;) So copying from the mailing list: > I have the updated the method documentation (also changed the method name to > toggleNodeOrientation() to clearly reflect > what it does) good move! > There would always be different ways to test the same thing. I think, current > test is in good shape and suffices the > purpose of unit testing this fix. > yeah, I agree - with a minor matter: the dynamic change testing is for a single key combination only. Personally, I prefer test completeness to really grab all changes (here all key combinations): future maintainers sometimes have ideas (f.i. inlining the isRTL, and accidentally not doing it on the simple keys) which they should be protected from doing - up to you, though. Assuming that Behaviours will move into public scope, shouldn't public/protected api documented when added (as isRtL())? Not sure about fx team internal conventions. Anyway, all side-thought, so feel free to commit ------------- PR: https://git.openjdk.java.net/jfx/pull/114
