CPLAT-6104 Add over_react_redux example app #439

aaronlademann-wf · 2019-12-10T21:24:28Z

These changes add a full-featured example application within app/over_react_redux/todo_client that showcases what the over_react_redux lib can do!

To try out the app... pull down this branch. Then...

cd app/over_react_redux/todo_client
pub get
webdev serve

then navigate to http://localhost:8080

FYA @greglittlefield-wf @joebingham-wk @kealjones-wk @sydneyjodon-wk

aviary-wf · 2019-12-10T21:24:52Z

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.

+ This is what dartlang expects, and we get docked for not doing it this way when pana analyzes the repo.

joebingham-wk

This app is awesome. The components are all super elegant and there's lots of cool stuff (utilities / techniques) in there. I'm very excited to have this be the example for a Redux app.

I found a few nitpicky things. Given that it's an example app pretty much all of it is just a suggestion and there aren't any show stoppers. Thank you for working so hard on this!

app/over_react_redux/todo_client/lib/src/utils.dart

app/over_react_redux/todo_client/smithy.yml

app/over_react_redux/todo_client/web/js/material-ui-config.js

app/over_react_redux/todo_client/lib/src/components/shared/list_item_mixin.dart

app/over_react_redux/todo_client/lib/src/components/shared/material_ui.dart

joebingham-wk · 2019-12-18T17:24:57Z

app/over_react_redux/todo_client/lib/src/components/shared/menu_overlay.dart

+    );
+  }
+
+  // TODO: Look into the cloneElement issue with the missing/problematic SyntheticEvent wrapper that forces us to use dynamic here


It might be nice to make a ticket for this and reference it here!

app/over_react_redux/todo_client/lib/src/components/shared/todo_item_text_field.dart

app/over_react_redux/todo_client/lib/src/components/user_selector.dart

…eact_redux_example_app

+ Use extension methods! + Fix some problematic comparison logic + Simplify top level traversal

joebingham-wk

Looks really good, just a couple small things!

app/over_react_redux/todo_client/lib/src/components/shared/list_item_mixin.dart

app/over_react_redux/todo_client/pubspec.yaml

greglittlefield-wf

Done with first pass, looks awesome overall though!

app/over_react_redux/todo_client/.gitignore

greglittlefield-wf · 2019-12-18T23:08:39Z

app/over_react_redux/todo_client/.gitignore

+.packages
+.pub
+packages
+*.over_react.g.dart


Why are these ignored? This package is build_to: source so it shouldn't need them

I think you're thinking about the over_react package @greglittlefield-wf - this app is its own package - which does not specify build_to: source.

Whoops, sorry, I meant build_to: cache. I always get those mixed up 😓

But yeah, since they're cache, they should only ever exist inside the .dart_tool directory within this repo.

Bump on this.

app/over_react_redux/todo_client/lib/src/components/app.dart

app/over_react_redux/todo_client/lib/src/components/shared/avatar_with_colors.dart

app/over_react_redux/todo_client/lib/src/components/user_selector.dart

app/over_react_redux/todo_client/lib/src/store.dart

app/over_react_redux/todo_client/lib/src/utils.dart

app/over_react_redux/todo_client/lib/src/components/app_bar/app_bar_local_storage_menu.dart

dave-doty · 2019-12-19T02:16:35Z

It would be super helpful if this example app also had a few unit tests (and possibly some kind of end-to-end tests), to show suggested examples how to do unit testing of reducer logic, components, and whatever else. (middleware?)

+ Since we’re using extension methods

+ Apparently this was added in built_redux 7.5.9

greglittlefield-wf · 2020-01-16T16:54:25Z

app/over_react_redux/todo_client/.gitignore

+.packages
+.pub
+packages
+*.over_react.g.dart


Bump on this.

greglittlefield-wf · 2020-01-16T17:06:10Z

app/over_react_redux/todo_client/lib/src/components/shared/redraw_counter_component_mixin.dart

+
+  @override
+  @mustCallSuper
+  void componentDidUpdate(_, __, [___]) {


Any mixin that overrides lifecycle methods should always call super, otherwise lifecycle overrides from parent classes or other mixins won't get called.

app/over_react_redux/todo_client/lib/src/components/create_input.dart

greglittlefield-wf · 2020-01-16T17:19:42Z

app/over_react_redux/todo_client/lib/src/components/user_selector.dart

@@ -18,11 +18,12 @@ UiFactory<UserSelectorProps> ConnectedUserSelector = connect<AppState, UserSelec
    mapStateToPropsWithOwnProps: (state, ownProps) {
      return (UserSelector()
        ..selectedUser = ownProps.selectedUserId.isNotEmpty
-            ? state.users.firstWhere((user) => user.id == ownProps.selectedUserId)
+            ? state.users.singleWhere((user) => user.id == ownProps.selectedUserId, orElse: () => null)


#FYI singleWhere iterates the full list to validate there are no other matches, whereas firstWhere short-circuits, so the latter may be preferable in perf-sensitive areas where you can assume there won't be dupes.

app/over_react_redux/todo_client/lib/src/store.dart

greglittlefield-wf · 2020-01-16T17:39:19Z

app/over_react_redux/todo_client/test/unit/browser/components/fixtures/utils.dart

+  if (!muiJsIsAvailable()) return;
+}
+
+JsBackedMap getJsProps(Ref ref) {


This seems to be unused

…pure

joebingham-wk

The new tests look awesome! I think it establishes a really simple and effective pattern, and the redraw mixin in dope.

Pretty much everything I caught was super little nit picky - feel free to resolve any of those that don't seem worth worrying about. Otherwise I just had a couple small questions

app/over_react_redux/todo_client/lib/src/components/todo_list.dart

app/over_react_redux/todo_client/lib/src/components/todo_list_item.dart

app/over_react_redux/todo_client/lib/src/components/user_list.dart

app/over_react_redux/todo_client/lib/src/components/user_list_item.dart

app/over_react_redux/todo_client/lib/src/components/user_selector.dart

app/over_react_redux/todo_client/test/unit/browser/redux/store_test.dart

…loads

Localstorage reducer/middleware cleanup

joebingham-wk

Code looks awesome!
Ran the app and tested functionality
Used devtools to inspect updates

+10

joebingham-wk · 2020-01-21T23:58:09Z

app/over_react_redux/todo_client/test/unit/browser/components/fixtures/utils.dart

+import 'dart:js';
+
+import 'package:over_react/over_react.dart';
+import 'package:react/react_client/react_interop.dart';


#nit unused import

Suggested change

import 'package:react/react_client/react_interop.dart';

greglittlefield-wf

+1

Again, awesome work on this @aaronlademann-wf!!

greglittlefield-wf · 2020-01-23T21:25:02Z

@Workiva/release-management-p

rmconsole-wf

+1 from RM

rmconsole-wf added the Merge Requirements Unmet label Dec 10, 2019

aaronlademann-wf force-pushed the CPLAT-6104_over_react_redux_example_app branch from 34535a2 to a3c24d3 Compare December 11, 2019 00:03

aaronlademann-wf added 2 commits December 10, 2019 17:03

Use main.dart for example entrypoint(s)

b946942

+ This is what dartlang expects, and we get docked for not doing it this way when pana analyzes the repo.

Split up existing examples into sub-directories

1f1b5d7

aaronlademann-wf force-pushed the CPLAT-6104_over_react_redux_example_app branch 5 times, most recently from 6eef3ce to 5dc886a Compare December 11, 2019 19:55

Add over_react_redux todo example app

2fe3179

aaronlademann-wf force-pushed the CPLAT-6104_over_react_redux_example_app branch from 5dc886a to 2fe3179 Compare December 12, 2019 15:22

aaronlademann-wf marked this pull request as ready for review December 12, 2019 19:57

joebingham-wk requested changes Dec 18, 2019

View reviewed changes

aaronlademann-wf added 6 commits December 18, 2019 13:13

Merge remote-tracking branch 'upstream/master' into CPLAT-6104_over_r…

3b185b3

…eact_redux_example_app

Refine recursive JsBackedMap stuff in todo_client example

5b2f736

+ Use extension methods! + Fix some problematic comparison logic + Simplify top level traversal

Improve variable names

7a8b09b

Remove unnecessary build tooling config

3a32887

Remove unused js config

39d44ab

Specify requiredProps

9525ea8

aaronlademann-wf requested a review from joebingham-wk December 18, 2019 20:30

joebingham-wk reviewed Dec 18, 2019

View reviewed changes

greglittlefield-wf reviewed Dec 19, 2019

View reviewed changes

app/over_react_redux/todo_client/lib/src/components/app_bar/app_bar_local_storage_menu.dart Outdated Show resolved Hide resolved

aaronlademann-wf added 5 commits December 19, 2019 13:16

Bump SDK constraint for example app

e39071a

+ Since we’re using extension methods

Remove unnecessary white space

372fc5d

Update .gitignore for example app

c083a32

Use variadic children consistently in the example app

56b39f2

Don’t use instance state here

2127adc

rmconsole-wf removed the Merge Conflicts label Jan 15, 2020

Update generated file

fd5b04b

+ Apparently this was added in built_redux 7.5.9

greglittlefield-wf reviewed Jan 16, 2020

View reviewed changes

Move local storage side manipulation into middleware to make reducer …

00a1cd0

…pure

joebingham-wk reviewed Jan 16, 2020

View reviewed changes

greglittlefield-wf and others added 6 commits January 16, 2020 15:13

Remove unused type from actions, simplify JSON representation and pay…

73f2538

…loads

Fix save menu not detecting changes

c2ec75a

Fix "Save As" not working for default value

dd01aec

Simplify LocalStorage initialization

863df28

Merge pull request #451 from Workiva/localstorage-cleanup

6b72ee6

Localstorage reducer/middleware cleanup

Address CR feedback

5a84e2a

aaronlademann-wf mentioned this pull request Jan 21, 2020

Proxy event.persist() when wrapping synthetic JS events Workiva/react-dart#249

Closed

joebingham-wk previously approved these changes Jan 22, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jan 22, 2020

aaronlademann-wf dismissed joebingham-wk’s stale review via c87e308 January 22, 2020 23:58

rmconsole-wf added Merge Requirements Unmet and removed Merge Requirements Met labels Jan 22, 2020

aaronlademann-wf force-pushed the CPLAT-6104_over_react_redux_example_app branch from c87e308 to 5a84e2a Compare January 23, 2020 20:12

greglittlefield-wf approved these changes Jan 23, 2020

View reviewed changes

rmconsole-wf added Merge Requirements Met and removed Merge Requirements Unmet labels Jan 23, 2020

rmconsole-wf added the RM Ready label Jan 23, 2020

rmconsole-wf approved these changes Jan 23, 2020

View reviewed changes

rm-astro-wf merged commit e06a3a3 into master Jan 23, 2020

rm-astro-wf deleted the CPLAT-6104_over_react_redux_example_app branch January 23, 2020 21:25

This was referenced Jan 29, 2020

RM-63989 RM-63988 Release over_react 3.1.8 #455

Closed

RM-63989 RM-63988 RM-60994 RM-60995 Release over_react 3.2.0 #456

Merged

joebingham-wk mentioned this pull request Feb 17, 2020

CPLAT-8693 Finalize connect for w_flux #459

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CPLAT-6104 Add over_react_redux example app #439

CPLAT-6104 Add over_react_redux example app #439

aaronlademann-wf commented Dec 10, 2019 •

edited

Loading

aviary-wf commented Dec 10, 2019

joebingham-wk left a comment

joebingham-wk Dec 18, 2019

joebingham-wk left a comment

greglittlefield-wf left a comment

greglittlefield-wf Dec 18, 2019

aaronlademann-wf Dec 19, 2019

greglittlefield-wf Dec 19, 2019

greglittlefield-wf Jan 16, 2020

dave-doty commented Dec 19, 2019

greglittlefield-wf Jan 16, 2020

greglittlefield-wf Jan 16, 2020

greglittlefield-wf Jan 16, 2020

greglittlefield-wf Jan 16, 2020

joebingham-wk left a comment

joebingham-wk left a comment

joebingham-wk Jan 21, 2020

greglittlefield-wf left a comment

greglittlefield-wf commented Jan 23, 2020

rmconsole-wf left a comment

CPLAT-6104 Add over_react_redux example app #439

CPLAT-6104 Add over_react_redux example app #439

Conversation

aaronlademann-wf commented Dec 10, 2019 • edited Loading

aviary-wf commented Dec 10, 2019

Security Insights

Action Items

joebingham-wk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joebingham-wk left a comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dave-doty commented Dec 19, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joebingham-wk left a comment

Choose a reason for hiding this comment

joebingham-wk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglittlefield-wf left a comment

Choose a reason for hiding this comment

greglittlefield-wf commented Jan 23, 2020

rmconsole-wf left a comment

Choose a reason for hiding this comment

aaronlademann-wf commented Dec 10, 2019 •

edited

Loading