> On Sept. 27, 2017, 8:35 p.m., Kai Huang wrote: > > ui/src/main/js/components/Pagination.js > > Lines 93 (patched) > > <https://reviews.apache.org/r/62607/diff/3/?file=1837592#file1837592line93> > > > > Should this be extracted into renderer? So that we don't need to pass > > isTable flags? > > David McLaughlin wrote: > I added a comment that describes why the user must provide this. You > cannot return a list of elements - you have to return a single container. So > Pagination needs to know what to wrap your items with. > > Joshua Cohen wrote: > Another "fwiw", I think in React 16 you *can* return a list of elements: > https://facebook.github.io/react/blog/2017/09/26/react-v16.0.html#new-render-return-types-fragments-and-strings > (note I haven't actually reviewed the code in question to see whether or not > it makes sense in this situation).
Ah, good to know - that's an excellent addition. In this case I do still need to know it's a table so I can wrap the pagination navigation in a tr, so will leave as-is. But I'll make sure the comment is modified to reflect this in a follow-up review. - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/62607/#review186469 ----------------------------------------------------------- On Sept. 27, 2017, 9:31 p.m., David McLaughlin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/62607/ > ----------------------------------------------------------- > > (Updated Sept. 27, 2017, 9:31 p.m.) > > > Review request for Aurora, Kai Huang and Santhosh Kumar Shanmugham. > > > Repository: aurora > > > Description > ------- > > Replace Preact and custom testing with React + jest + Enzyme now that they > are MIT licensed. > > Side-effects: > > * The Reactable library is not compatible with React 16.0 (the MIT licensed > version we are forced to use) and attempts to flag this started as early as > June this year. But the author seems to have abandoned the library. So I had > to create my own Pagination class. I found myself repeatedly using Reactable > in places where I just wanted pagination (for lists of divs) anyway, so maybe > this is a good thing. > * Deleted custom (and questionable) shallow renderer. > * The download time will be reduced due to not having to download PhantomJS. > > > Diffs > ----- > > build.gradle 460500aaeab28dcfeb29ec602d057ea4bca12378 > ui/karma.conf.js d42cda7d025f8c5618039888f4fb49ff388c3af1 > ui/package.json d6802029e0aeb9f6692ec8cc065949ce17b2ca07 > ui/src/__mocks__/react.js PRE-CREATION > ui/src/main/js/components/Pagination.js PRE-CREATION > ui/src/main/js/components/RoleList.js > 32595601aae06730f537d37d95be2e746f779103 > ui/src/main/js/components/__tests__/Breadcrumb-test.js > 18af0fe6c360f375fb0fa1694304d821041d9e16 > ui/src/main/js/components/__tests__/Home-test.js > 8e6bc09050148ea9711dbfe6f52b83c8210262be > ui/src/main/js/components/__tests__/Pagination-test.js PRE-CREATION > ui/src/main/js/pages/__tests__/Home-test.js > 4f13f99ea24c0252b5f6cbb6980e0c96832d5120 > ui/src/main/js/utils/ShallowRender.js > 52e8bb259264e7dd47ee97cec35553acc147e818 > ui/src/main/js/utils/__tests__/ShallowRender-test.js > d5663a7c44de795a5aa0b28d4f373871ffcc3fd2 > ui/src/main/sass/components/_tables.scss > 58f176cffcdae34de7c776e31b3f0342efa3024f > ui/test-setup.js PRE-CREATION > ui/webpack.config.js e7cd672df62006bd5c5b60a6ba0903e16a767d13 > > > Diff: https://reviews.apache.org/r/62607/diff/4/ > > > Testing > ------- > > $ ./gradlew ui:lint > $ ./gradlew ui:test > > I also tested the UI in my Vagrant image. > > > Thanks, > > David McLaughlin > >
