You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by David McLaughlin <da...@dmclaughlin.com> on 2017/09/08 23:40:18 UTC
Review Request 62135: HomePage implemented in Preact
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/
-----------------------------------------------------------
Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
Repository: aurora
Description
-------
Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
If Preact gets a shallow renderer in the future, we can swap the one included here out.
Diffs
-----
src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
ui/src/main/js/client/scheduler-client.js PRE-CREATION
ui/src/main/js/components/Breadcrumb.js PRE-CREATION
ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
ui/src/main/js/components/Icon.js PRE-CREATION
ui/src/main/js/components/Loading.js PRE-CREATION
ui/src/main/js/components/Navigation.js PRE-CREATION
ui/src/main/js/components/RoleList.js PRE-CREATION
ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
ui/src/main/js/pages/Home.js PRE-CREATION
ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
ui/src/main/js/utils/ShallowRender.js PRE-CREATION
ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
ui/src/main/sass/app.scss PRE-CREATION
ui/src/main/sass/components/_base.scss PRE-CREATION
ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
ui/src/main/sass/components/_home-page.scss PRE-CREATION
ui/src/main/sass/components/_layout.scss PRE-CREATION
ui/src/main/sass/components/_navigation.scss PRE-CREATION
ui/src/main/sass/components/_tables.scss PRE-CREATION
ui/src/main/sass/modules/_all.scss PRE-CREATION
ui/src/main/sass/modules/_colors.scss PRE-CREATION
ui/src/main/sass/modules/_typography.scss PRE-CREATION
Diff: https://reviews.apache.org/r/62135/diff/1/
Testing
-------
./gradlew ui:test
Running in vagrant (screenshots attached).
File Attachments
----------------
preview
https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
Thanks,
David McLaughlin
Re: Review Request 62135: HomePage implemented in Preact
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185193
-----------------------------------------------------------
Ship it!
Ship It!
- Kai Huang
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 11, 2017, 6:22 p.m., Kai Huang wrote:
> > ui/src/main/js/utils/__tests__/ShallowRender-test.js
> > Lines 44 (patched)
> > <https://reviews.apache.org/r/62135/diff/1/?file=1818858#file1818858line44>
> >
> > nit. Add the following assertion as well, since the name is tesing handlie multiple elements?
> > ```
> > expect(el.contains(<Leaf name='jon' />)).to.be.true;
> > ```
Done.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185096
-----------------------------------------------------------
On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 5:44 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/2/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185096
-----------------------------------------------------------
ui/src/main/js/utils/__tests__/ShallowRender-test.js
Lines 44 (patched)
<https://reviews.apache.org/r/62135/#comment261325>
nit. Add the following assertion as well, since the name is tesing handlie multiple elements?
```
expect(el.contains(<Leaf name='jon' />)).to.be.true;
```
- Kai Huang
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 11, 2017, 4:37 p.m., Kai Huang wrote:
> > ui/src/main/js/index.js
> > Line 5 (original), 5 (patched)
> > <https://reviews.apache.org/r/62135/diff/1/?file=1818854#file1818854line5>
> >
> > Dumb question: I didn't see SchedulerClient defined in client/scheduler-client.js. Is it implicitly provided by the makeClient() function?
The key line is this:
export default makeClient();
makeClient() returns the ReadOnlySchedulerClient. So it's basically just exporting a constant.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185062
-----------------------------------------------------------
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185062
-----------------------------------------------------------
ui/src/main/js/index.js
Line 5 (original), 5 (patched)
<https://reviews.apache.org/r/62135/#comment261283>
Dumb question: I didn't see SchedulerClient defined in client/scheduler-client.js. Is it implicitly provided by the makeClient() function?
- Kai Huang
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 11, 2017, 4:18 p.m., Joshua Cohen wrote:
> > lgtm overall. One question: do you think it's worthwhile to extract the shallow render to its own npm module that we depend on? Maybe that would make it possible/easier to have it upstreamed to preact? No idea if there are any challenges to that from an OSS perspective (i.e. we're ok to contribute to Aurora, but creating a new OSS project/publishing to npm/etc. may need an internal OSS review at Twitter?).
I think there's probably better ways to do it if I had the time to learn the internals of Preact. The major downside to this approach is that it's comparing a rendered DOM tree to a vnode tree. The way it's done in React is *much* cleaner, where you're comparing vnode tree to vnode tree and can just use basic equality and tree traversal techniques.
One of the goals of Preact is to be super lightweight, and for that reason they couple their rendering directly to the DOM, which is why the shallow rendering here plugs into that. There is a String renderer (https://github.com/developit/preact-render-to-string) that exposes a shallowRender method, but you end up comparing properties like Object[object object] when you have complex properties and I ended up with a lot of false positive tests. I haven't been able to figure out how much it would take to create a vnode render, but I was put off by the TODO in the preact-test-utils library (https://github.com/windyGex/preact-test-utils/blob/master/src/index.js#L6). The author implies in one of the github issuess that implementing shallow rendering is very complex in Preact. I'm going to keep an eye out though in the hopes that someone steps up and does it cleanly there.. that library is the basis for enzyme compatibility. If I ever have time, I may even take a crack at it myself!
> On Sept. 11, 2017, 4:18 p.m., Joshua Cohen wrote:
> > ui/src/main/js/components/Breadcrumb.js
> > Lines 12 (patched)
> > <https://reviews.apache.org/r/62135/diff/1/?file=1818846#file1818846line12>
> >
> > This is a change for the current behavior of the breadcrumbs in that currently the last crumb is not clickable. Is this the intent? I think the current behavior is preferable, but don't feel super strongly about it.
> >
> > Here's how I handled it in the original react prototype (which I remember feeling was kind of hacky truth be told): https://github.com/jcohen/aurora-react/blob/master/src/components/Breadcrumbs.js
Changing current behavior wasn't intentional, I kinda just started from scratch and based it on the github breadcrumb here: https://github.com/apache/aurora
I don't have strong opinions about whether or not to change it, but one benefit is that on more complex pages where we can modify the URL, it was nice to have a reset button right there in the navigation.
> On Sept. 11, 2017, 4:18 p.m., Joshua Cohen wrote:
> > ui/src/main/js/components/Home.js
> > Line 4 (original), 3-5 (patched)
> > <https://reviews.apache.org/r/62135/diff/1/?file=1818847#file1818847line4>
> >
> > Just curious, why the move away from the ES6 style?
Because of how Preact attaches names to custom components, the shallow rendering hack needs an explicit name for the equality matching to work properly. You can still this with ES6 style, but AFAICT you need the export default on a separate line?
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185082
-----------------------------------------------------------
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Joshua Cohen <jc...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185082
-----------------------------------------------------------
Ship it!
lgtm overall. One question: do you think it's worthwhile to extract the shallow render to its own npm module that we depend on? Maybe that would make it possible/easier to have it upstreamed to preact? No idea if there are any challenges to that from an OSS perspective (i.e. we're ok to contribute to Aurora, but creating a new OSS project/publishing to npm/etc. may need an internal OSS review at Twitter?).
ui/src/main/js/components/Breadcrumb.js
Lines 12 (patched)
<https://reviews.apache.org/r/62135/#comment261299>
This is a change for the current behavior of the breadcrumbs in that currently the last crumb is not clickable. Is this the intent? I think the current behavior is preferable, but don't feel super strongly about it.
Here's how I handled it in the original react prototype (which I remember feeling was kind of hacky truth be told): https://github.com/jcohen/aurora-react/blob/master/src/components/Breadcrumbs.js
ui/src/main/js/components/Home.js
Line 4 (original), 3-5 (patched)
<https://reviews.apache.org/r/62135/#comment261300>
Just curious, why the move away from the ES6 style?
- Joshua Cohen
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 13, 2017, 5:51 p.m., Aurora ReviewBot wrote:
> > This patch does not apply cleanly against master (3ec0430), do you need to rebase?
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
Weird error. This definitely applies cleanly:
$ git checkout master
$ git pull origin master
Already up-to-date.
$ ./rbt patch -c 62135
Successfully apllied patch.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185314
-----------------------------------------------------------
On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 5:44 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/2/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185314
-----------------------------------------------------------
This patch does not apply cleanly against master (3ec0430), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Sept. 13, 2017, 5:44 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 5:44 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/2/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 13, 2017, 6:13 p.m., Aurora ReviewBot wrote:
> > This patch does not apply cleanly against master (3ec0430), do you need to rebase?
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
ReviewBot is broken. Ignoring this error and manually testing then pushing.
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185317
-----------------------------------------------------------
On Sept. 13, 2017, 6:07 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 6:07 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/3/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
> On Sept. 13, 2017, 6:13 p.m., Aurora ReviewBot wrote:
> > This patch does not apply cleanly against master (3ec0430), do you need to rebase?
> >
> > I will refresh this build result if you post a review containing "@ReviewBot retry"
>
> David McLaughlin wrote:
> ReviewBot is broken. Ignoring this error and manually testing then pushing.
This is the error that is throwing ReviewBot:
https://builds.apache.org/job/AuroraBot/116591/console
HEAD is now at 3ec0430 Fix concurrency issues around banned offers in HostOffers
error: missing binary patch data for 'src/main/resources/scheduler/assets/images/aurora_logo_white.png'
error: binary patch does not apply to 'src/main/resources/scheduler/assets/images/aurora_logo_white.png'
Falling back to three-way merge...
error: missing binary patch data for 'src/main/resources/scheduler/assets/images/aurora_logo_white.png'
error: binary patch does not apply to 'src/main/resources/scheduler/assets/images/aurora_logo_white.png'
error: src/main/resources/scheduler/assets/images/aurora_logo_white.png: patch does not apply
- David
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185317
-----------------------------------------------------------
On Sept. 13, 2017, 6:07 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 6:07 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/3/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185317
-----------------------------------------------------------
This patch does not apply cleanly against master (3ec0430), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Sept. 13, 2017, 9:07 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 9:07 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/3/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185324
-----------------------------------------------------------
This patch does not apply cleanly against master (3ec0430), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Sept. 13, 2017, 2:22 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 13, 2017, 2:22 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/4/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/
-----------------------------------------------------------
(Updated Sept. 13, 2017, 6:22 p.m.)
Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
Changes
-------
Fix lint breakage.
Repository: aurora
Description
-------
Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
If Preact gets a shallow renderer in the future, we can swap the one included here out.
Diffs (updated)
-----
src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
ui/src/main/js/client/scheduler-client.js PRE-CREATION
ui/src/main/js/components/Breadcrumb.js PRE-CREATION
ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
ui/src/main/js/components/Icon.js PRE-CREATION
ui/src/main/js/components/Loading.js PRE-CREATION
ui/src/main/js/components/Navigation.js PRE-CREATION
ui/src/main/js/components/RoleList.js PRE-CREATION
ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
ui/src/main/js/pages/Home.js PRE-CREATION
ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
ui/src/main/js/utils/ShallowRender.js PRE-CREATION
ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
ui/src/main/sass/app.scss PRE-CREATION
ui/src/main/sass/components/_base.scss PRE-CREATION
ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
ui/src/main/sass/components/_home-page.scss PRE-CREATION
ui/src/main/sass/components/_layout.scss PRE-CREATION
ui/src/main/sass/components/_navigation.scss PRE-CREATION
ui/src/main/sass/components/_tables.scss PRE-CREATION
ui/src/main/sass/modules/_all.scss PRE-CREATION
ui/src/main/sass/modules/_colors.scss PRE-CREATION
ui/src/main/sass/modules/_typography.scss PRE-CREATION
Diff: https://reviews.apache.org/r/62135/diff/4/
Changes: https://reviews.apache.org/r/62135/diff/3-4/
Testing
-------
./gradlew ui:test
Running in vagrant (screenshots attached).
File Attachments
----------------
preview
https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
Thanks,
David McLaughlin
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/
-----------------------------------------------------------
(Updated Sept. 13, 2017, 6:07 p.m.)
Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
Changes
-------
@ReviewBot retry
Repository: aurora
Description
-------
Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
If Preact gets a shallow renderer in the future, we can swap the one included here out.
Diffs (updated)
-----
src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
ui/src/main/js/client/scheduler-client.js PRE-CREATION
ui/src/main/js/components/Breadcrumb.js PRE-CREATION
ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
ui/src/main/js/components/Icon.js PRE-CREATION
ui/src/main/js/components/Loading.js PRE-CREATION
ui/src/main/js/components/Navigation.js PRE-CREATION
ui/src/main/js/components/RoleList.js PRE-CREATION
ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
ui/src/main/js/pages/Home.js PRE-CREATION
ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
ui/src/main/js/utils/ShallowRender.js PRE-CREATION
ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
ui/src/main/sass/app.scss PRE-CREATION
ui/src/main/sass/components/_base.scss PRE-CREATION
ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
ui/src/main/sass/components/_home-page.scss PRE-CREATION
ui/src/main/sass/components/_layout.scss PRE-CREATION
ui/src/main/sass/components/_navigation.scss PRE-CREATION
ui/src/main/sass/components/_tables.scss PRE-CREATION
ui/src/main/sass/modules/_all.scss PRE-CREATION
ui/src/main/sass/modules/_colors.scss PRE-CREATION
ui/src/main/sass/modules/_typography.scss PRE-CREATION
Diff: https://reviews.apache.org/r/62135/diff/3/
Changes: https://reviews.apache.org/r/62135/diff/2-3/
Testing
-------
./gradlew ui:test
Running in vagrant (screenshots attached).
File Attachments
----------------
preview
https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
Thanks,
David McLaughlin
Re: Review Request 62135: HomePage implemented in Preact
Posted by David McLaughlin <da...@dmclaughlin.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/
-----------------------------------------------------------
(Updated Sept. 13, 2017, 5:44 p.m.)
Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
Changes
-------
Add extra assertion in test.
Repository: aurora
Description
-------
Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
If Preact gets a shallow renderer in the future, we can swap the one included here out.
Diffs (updated)
-----
src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
ui/src/main/js/client/scheduler-client.js PRE-CREATION
ui/src/main/js/components/Breadcrumb.js PRE-CREATION
ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
ui/src/main/js/components/Icon.js PRE-CREATION
ui/src/main/js/components/Loading.js PRE-CREATION
ui/src/main/js/components/Navigation.js PRE-CREATION
ui/src/main/js/components/RoleList.js PRE-CREATION
ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
ui/src/main/js/pages/Home.js PRE-CREATION
ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
ui/src/main/js/utils/ShallowRender.js PRE-CREATION
ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
ui/src/main/sass/app.scss PRE-CREATION
ui/src/main/sass/components/_base.scss PRE-CREATION
ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
ui/src/main/sass/components/_home-page.scss PRE-CREATION
ui/src/main/sass/components/_layout.scss PRE-CREATION
ui/src/main/sass/components/_navigation.scss PRE-CREATION
ui/src/main/sass/components/_tables.scss PRE-CREATION
ui/src/main/sass/modules/_all.scss PRE-CREATION
ui/src/main/sass/modules/_colors.scss PRE-CREATION
ui/src/main/sass/modules/_typography.scss PRE-CREATION
Diff: https://reviews.apache.org/r/62135/diff/2/
Changes: https://reviews.apache.org/r/62135/diff/1-2/
Testing
-------
./gradlew ui:test
Running in vagrant (screenshots attached).
File Attachments
----------------
preview
https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
Thanks,
David McLaughlin
Re: Review Request 62135: HomePage implemented in Preact
Posted by Aurora ReviewBot <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185037
-----------------------------------------------------------
This patch does not apply cleanly against master (3ec0430), do you need to rebase?
I will refresh this build result if you post a review containing "@ReviewBot retry"
- Aurora ReviewBot
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>
Re: Review Request 62135: HomePage implemented in Preact
Posted by Kai Huang <te...@hotmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/62135/#review185088
-----------------------------------------------------------
ui/src/main/js/components/Home.js
Line 4 (original), 3-5 (patched)
<https://reviews.apache.org/r/62135/#comment261308>
Don't have a strong preference on one over another, as long as we keep consistent. In general it seems reasonable to use named functions and avoid anonymous ones.
- Kai Huang
On Sept. 8, 2017, 11:40 p.m., David McLaughlin wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62135/
> -----------------------------------------------------------
>
> (Updated Sept. 8, 2017, 11:40 p.m.)
>
>
> Review request for Aurora, Joshua Cohen, Kai Huang, and Santhosh Kumar Shanmugham.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Implementation of the home page in PreactJS. This was delayed heavily by the lack of a shallow renderer (https://facebook.github.io/react/docs/shallow-renderer.html) for testing. It was too painful to test without it (e.g. Links when fully rendered must be rendered within a Router context), so I hand-rolled one based on some community attempts. The key motivation is just to allow us to decouple markup from component logic, and also to avoid having to test child components within component heirarchies. IMO the end result is very clean and easy to read (and write!).
>
> If Preact gets a shallow renderer in the future, we can swap the one included here out.
>
>
> Diffs
> -----
>
> src/main/resources/scheduler/assets/images/aurora_logo_white.png PRE-CREATION
> ui/.eslintrc 355b6a8a5f5d25dd4c71dc546dc7d4acfffdd506
> ui/package.json f712518b27477bccb03f49c86eac3ee5f769fc88
> ui/src/main/js/client/scheduler-client.js PRE-CREATION
> ui/src/main/js/components/Breadcrumb.js PRE-CREATION
> ui/src/main/js/components/Home.js 91d60b387cf3b1fb268e73b7b50922a83898c31f
> ui/src/main/js/components/Icon.js PRE-CREATION
> ui/src/main/js/components/Loading.js PRE-CREATION
> ui/src/main/js/components/Navigation.js PRE-CREATION
> ui/src/main/js/components/RoleList.js PRE-CREATION
> ui/src/main/js/components/__tests__/Breadcrumb-test.js PRE-CREATION
> ui/src/main/js/components/__tests__/Home-test.js 2a80958d9303d1d3b9ae8f95013c66cb39f6bac3
> ui/src/main/js/index.js 2f7467b41ac3373a90c5453a7534d384a585b464
> ui/src/main/js/pages/Home.js PRE-CREATION
> ui/src/main/js/pages/__tests__/Home-test.js PRE-CREATION
> ui/src/main/js/utils/ShallowRender.js PRE-CREATION
> ui/src/main/js/utils/__tests__/ShallowRender-test.js PRE-CREATION
> ui/src/main/sass/app.scss PRE-CREATION
> ui/src/main/sass/components/_base.scss PRE-CREATION
> ui/src/main/sass/components/_breadcrumb.scss PRE-CREATION
> ui/src/main/sass/components/_home-page.scss PRE-CREATION
> ui/src/main/sass/components/_layout.scss PRE-CREATION
> ui/src/main/sass/components/_navigation.scss PRE-CREATION
> ui/src/main/sass/components/_tables.scss PRE-CREATION
> ui/src/main/sass/modules/_all.scss PRE-CREATION
> ui/src/main/sass/modules/_colors.scss PRE-CREATION
> ui/src/main/sass/modules/_typography.scss PRE-CREATION
>
>
> Diff: https://reviews.apache.org/r/62135/diff/1/
>
>
> Testing
> -------
>
> ./gradlew ui:test
>
> Running in vagrant (screenshots attached).
>
>
> File Attachments
> ----------------
>
> preview
> https://reviews.apache.org/media/uploaded/files/2017/09/08/011f99fb-a14e-4547-b90d-a5a1909c737c__Screen_Shot_2017-09-08_at_3.58.48_PM.png
>
>
> Thanks,
>
> David McLaughlin
>
>