CPLAT-4304 Expose React Fragments by joebingham-wk · Pull Request #303 · Workiva/over_react

joebingham-wk · 2019-06-13T20:43:47Z

~~Hold: This PR is reliant upon the interop introduced in react-dart#188 within react-dart. Until it is merged, the CI will fail.~~

Motivation

React 16 brings with it a common pattern in React for a component to return multiple elements. Fragments let you group a list of children without adding extra nodes to the DOM.

Changes

Add aFragment component that can be used to wrap React elements.
Update the Component2 examples to use Fragment where possible.
Write test coverage

Release Notes

Add the Fragment API.

Review

See CONTRIBUTING.md for more details on review types (+1 / QA +1 / +10) and code review process.

Please review:
@aaronlademann-wf @kealjones-wk @greglittlefield-wf @sydneyjodon-wk

QA Checklist

Tests were updated and provide good coverage of the changeset and other affected code
Manual testing was performed if needed
- Steps from PR author:
- Anything falling under manual testing criteria outlined in CONTRIBUTING.md

Merge Checklist

While we perform many automated checks before auto-merging, some manual checks are needed:

A Client Platform member has reviewed these changes
There are no unaddressed comments - this check can be automated if reviewers use the "Request Changes" feature
For release PRs - Version metadata in Rosie comment is correct

aviary-wf · 2019-06-13T20:44:09Z

Security Insights

No security relevant content was detected by automated scans.

Action Items

Review PR for security impact; comment "security review required" if needed or unsure
Verify aviary.yaml coverage of security relevant code

Questions or Comments? Reach out on Slack: #support-infosec.

…react into CPLAT-4304-react-fragments

kealjones-wk

Just a few nits, overall looks good.

kealjones-wk · 2019-06-17T19:47:20Z

pubspec.yaml

-      url: https://github.com/cleandart/react-dart.git
-      ref: 5.1.0-wip
+      url: git@github.com:cleandart/react-dart.git
+      ref: CPLAT-4304-react-fragments


Remember to change this back this before merges

web/component2/src/demos/button/button-examples.dart

kealjones-wk · 2019-06-17T19:50:32Z

web/component2/src/demos/progress/progress-basic.dart

 import '../../demo_components.dart';

-ReactElement progressBasicDemo() => Dom.div()(
+ReactElement progressBasicDemo() => Fragment()(


test/over_react/component/fragment_component_test.dart

kealjones-wk · 2019-06-17T19:54:55Z

test/over_react/component/fixtures/dummy_component.dart

+      ..addTestId('DummyButton')
+    )('oh hai');
+  }
+}


#nit always end files with a blank line

@sydneyjodon-wk this is a setting in WebStorm you can enable.

kealjones-wk · 2019-06-17T20:00:08Z

pubspec.yaml

    git:
-      url: https://github.com/cleandart/react-dart.git
-      ref: 5.1.0-wip
+      url: git@github.com:cleandart/react-dart.git


Suggested change

url: git@github.com:cleandart/react-dart.git

url: https://github.com/cleandart/react-dart.git

This should get CI working again

…react into CPLAT-4304-react-fragments

aaronlademann-wf

Some formatting issues that need to be addressed - but I thought running ddev format was already set up to use over_react_format?

lib/src/component/fragment_component.dart

test/over_react/component/fragment_component_test.dart

web/component2/src/demos/button/button-examples.dart

test/over_react/component/fragment_component_test.dart

lib/src/component/fragment_component.dart

aaronlademann-wf

Just some cleanup needed, then I'm ready to plus one @sydneyjodon-wk

aaronlademann-wf · 2019-06-19T12:53:34Z

lib/src/component/fragment_component.dart

+    implements builder_helpers.UiProps {
+  // Initialize to a JsBackedMap so that copying can be optimized
+  // when converting props during ReactElement creation.
+  // TODO 3.0.0-wip generate JsBackedMap-based implementation used when no backing map is provided, like we do for Component2


Seems like it is already doing this... so the TODO can be removed.

aaronlademann-wf · 2019-06-19T12:55:26Z

lib/src/component/fragment_component.dart

@@ -0,0 +1,31 @@
+import 'package:over_react/src/component_declaration/component_base.dart'


This file needs the copyright block:

// Copyright 2019 Workiva Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License.

aaronlademann-wf · 2019-06-19T12:55:55Z

test/over_react/component/fixtures/dummy_component.dart

@@ -0,0 +1,33 @@
+import 'package:over_react/over_react.dart';


This file needs the copyright block:

// Copyright 2019 Workiva Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at // // http://www.apache.org/licenses/LICENSE-2.0 // // Unless required by applicable law or agreed to in writing, software // distributed under the License is distributed on an "AS IS" BASIS, // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License.

aaronlademann-wf · 2019-06-19T12:56:04Z

test/over_react/component/fragment_component_test.dart

@@ -0,0 +1,70 @@
+// Copyright 2016 Workiva Inc.


Suggested change

// Copyright 2016 Workiva Inc.

// Copyright 2019 Workiva Inc.

aaronlademann-wf

+1

CPLAT-6244: DO NOT MERGE: Upgrade to analyzer 0.36.x and build_web_compilers 2.x

RM-53250 RM-53249 Release over_react 2.4.4+dart2

* master: Changelog entry for v2.4.4 over_react_2.4.4+dart2 Update base image to Dart2.4 Add dev_dep on build_resolvers. Remove coverage dep as it is currently unused. Upgrade to analyzer 0.36.x and build_web_compilers 2.x Changelog entry for v2.4.3 over_react_2.4.3+dart2 Run CI on the dev channel as well. Add more detailed warning message Fix another typo Fix typo Warn consumers if part directive is present, but no generations were found Update handler chain util tests. Reduce callback constant boilerplate Add explicit dynamic return types, use new typedef syntax Use local methods for better stack traces, add return type to fix lints Remove duplicated doc comments, which get inherited Remove CallbackUtil generic parameter, use covariant to add chainFromList typing

kealjones-wk · 2019-06-28T15:39:03Z

+10

aaronlademann-wf

+1

kealjones-wk · 2019-06-28T15:53:20Z

@Workiva/release-management-p

addressed

joebingham-wk added 3 commits June 13, 2019 10:20

Add initial wrapper

d8a837a

Add dependency to pubspec

1428798

Pull fragment from react.dart

6467eb4

rmconsole-wf added the Merge Requirements Unmet label Jun 13, 2019

joebingham-wk added the Hold Hold merges label Jun 13, 2019

sydneyjodon-wk and others added 3 commits June 14, 2019 16:22

Add tests

04b1cc4

Remove build_daemon

0ccd3b3

Merge branch 'CPLAT-4304-react-fragments' of github.com:Workiva/over_…

f486aa7

…react into CPLAT-4304-react-fragments

kealjones-wk requested changes Jun 17, 2019

View reviewed changes

kealjones-wk reviewed Jun 17, 2019

View reviewed changes

sydneyjodon-wk added 2 commits June 17, 2019 13:44

Update based on reviewer feedback

4e239e1

Merge branch 'CPLAT-4304-react-fragments' of github.com:Workiva/over_…

cd81ab0

…react into CPLAT-4304-react-fragments

aaronlademann-wf added this to the 3.1.0 milestone Jun 18, 2019

aaronlademann-wf added the Dependent PR label Jun 18, 2019

aaronlademann-wf requested changes Jun 18, 2019

View reviewed changes

greglittlefield-wf previously requested changes Jun 18, 2019

View reviewed changes

lib/src/component/fragment_component.dart Outdated Show resolved Hide resolved

sydneyjodon-wk added 3 commits June 18, 2019 15:00

Update comments

2d81710

Remove componentFactory argument

53c8a3b

Update formatting

b981614

aaronlademann-wf requested changes Jun 19, 2019

View reviewed changes

sydneyjodon-wk and others added 4 commits June 19, 2019 08:58

Update copyright comments

4151d76

Upgrade to analyzer 0.36.x and build_web_compilers 2.x

36493cb

Remove coverage dep as it is currently unused.

2c5d104

Add dev_dep on build_resolvers.

34c1543

aaronlademann-wf approved these changes Jun 24, 2019

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jun 24, 2019

evanweible-wf and others added 2 commits June 27, 2019 08:48

Update base image to Dart2.4

036128a

Merge pull request #310 from Workiva/dart24_bwc2

00cd90e

CPLAT-6244: DO NOT MERGE: Upgrade to analyzer 0.36.x and build_web_compilers 2.x

rmconsole-readonly-wk and others added 5 commits June 27, 2019 15:25

over_react_2.4.4+dart2

12e2898

Changelog entry for v2.4.4

388c7ea

Merge pull request #313 from Workiva/release_over_react_2.4.4+dart2

327f177

RM-53250 RM-53249 Release over_react 2.4.4+dart2

merge master and revert pubspec

019be24

Merge branch '3.1.0-wip' into CPLAT-4304-react-fragments

8292047

rmconsole-wf added Merge Conflicts Merge Requirements Unmet and removed Merge Requirements Met Merge Conflicts labels Jun 28, 2019

aaronlademann-wf removed Dependent PR Hold Hold merges labels Jun 28, 2019

aaronlademann-wf approved these changes Jun 28, 2019

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jun 28, 2019

rmconsole-wf added the RM Ready label Jun 28, 2019

rmconsole7-wk merged commit aa2d12e into 3.1.0-wip Jun 28, 2019

rmconsole7-wk deleted the CPLAT-4304-react-fragments branch June 28, 2019 15:53

	url: git@github.com:cleandart/react-dart.git
	url: https://github.com/cleandart/react-dart.git

		@@ -0,0 +1,31 @@
		import 'package:over_react/src/component_declaration/component_base.dart'

		@@ -0,0 +1,33 @@
		import 'package:over_react/over_react.dart';

	// Copyright 2016 Workiva Inc.
	// Copyright 2019 Workiva Inc.

Conversation

joebingham-wk commented Jun 13, 2019 • edited by aaronlademann-wf Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Changes

Release Notes

Review

QA Checklist

Merge Checklist

Uh oh!

aviary-wf commented Jun 13, 2019

Security Insights

Action Items

Uh oh!

kealjones-wk left a comment

Choose a reason for hiding this comment

Uh oh!

kealjones-wk Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronlademann-wf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aaronlademann-wf left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronlademann-wf left a comment

Choose a reason for hiding this comment

Uh oh!

kealjones-wk commented Jun 28, 2019

Uh oh!

aaronlademann-wf left a comment

Choose a reason for hiding this comment

Uh oh!

kealjones-wk commented Jun 28, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants

joebingham-wk commented Jun 13, 2019 •

edited by aaronlademann-wf

Loading

kealjones-wk Jun 17, 2019 •

edited

Loading