Skip to content

Commit

Permalink
Address CR feedback
Browse files Browse the repository at this point in the history
Revert formatting/content changes that shouldn't have been merged in
  • Loading branch information
clairesarsam-wf committed Jan 13, 2017
1 parent 08ce78d commit 8fb0ba0
Show file tree
Hide file tree
Showing 14 changed files with 115 additions and 242 deletions.
7 changes: 2 additions & 5 deletions lib/src/component/resize_sensor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
/// Thanks!
/// https://github.com/marcj/css-element-queries/blob/master/src/ResizeSensor.js
library resize_sensor;

import 'dart:collection';
import 'dart:html';

Expand Down Expand Up @@ -78,7 +79,6 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> {

Element _expandSensorChildRef;
Element _expandSensorRef;
Element _collapseSensorChildRef;
Element _collapseSensorRef;

@override
Expand Down Expand Up @@ -111,10 +111,7 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> {
..key = 'expandSensor'
)(expandSensorChild);

var collapseSensorChild = (Dom.div()
..ref = (ref) { _collapseSensorChildRef = ref; }
..style = _collapseSensorChildStyle
)();
var collapseSensorChild = (Dom.div()..style = _collapseSensorChildStyle)();

var collapseSensor = (Dom.div()
..className = 'resize-sensor-collapse'
Expand Down
32 changes: 14 additions & 18 deletions lib/src/component_declaration/component_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,20 +86,14 @@ typedef TProps UiFactory<TProps extends UiProps>([Map backingProps]);
/// For use as a Function variable type when the `backingProps` argument is not required.
typedef TProps BuilderOnlyUiFactory<TProps extends UiProps>();

typedef dynamic _RefTypedef(String ref);

/// The basis for a over_react component, extending [react.Component]. (Successor to [BaseComponent]).
/// The basis for a over_react component.
///
/// Includes support for strongly-typed props and utilities for prop and CSS classname forwarding.
///
/// Extends [react.Component].
///
/// Related: [UiStatefulComponent]
abstract class UiComponent<TProps extends UiProps> extends react.Component {
/// Returns the component of the specified [ref].
/// > `react.Component` if it is a Dart component
/// > DOM node if it is a DOM component.
///
/// Overridden for strong typing.
@override
_RefTypedef get ref => super.ref;

/// The props for the non-forwarding props defined in this component.
Iterable<ConsumedProps> get consumedProps => null;

Expand Down Expand Up @@ -131,12 +125,12 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component {
void validateRequiredProps(Map appliedProps) {
consumedProps?.forEach((ConsumedProps consumedProps) {
consumedProps.props.forEach((PropDescriptor prop) {
if (!prop.isRequired) return;
if (prop.isNullable && appliedProps.containsKey(prop.key)) return;
if (!prop.isNullable && appliedProps[prop.key] != null) return;
if (!prop.isRequired) return;
if (prop.isNullable && appliedProps.containsKey(prop.key)) return;
if (!prop.isNullable && appliedProps[prop.key] != null) return;

throw new PropError.required(prop.key, prop.errorMessage);
});
throw new PropError.required(prop.key, prop.errorMessage);
});
});
}

Expand Down Expand Up @@ -203,11 +197,11 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component {
// ----------------------------------------------------------------------
}

/// /// The basis for a stateful over_react component.
/// The basis for a stateful over_react component.
///
/// Includes support for strongly-typed props and state and utilities for prop and CSS classname forwarding.
///
/// Extends [react.Component]
/// Extends [react.Component].
///
/// Related: [UiComponent]
abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiState> extends UiComponent<TProps> {
Expand Down Expand Up @@ -240,10 +234,12 @@ abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiStat
set unwrappedState(Map value) => super.state = value;

/// Returns a typed state object backed by the specified [stateMap].
///
/// Required to properly instantiate the generic [TState] class.
TState typedStateFactory(Map stateMap);

/// Returns a typed state object backed by a new Map.
///
/// Convenient for use with [getInitialState] and [setState].
TState newState() => typedStateFactory({});

Expand Down
82 changes: 52 additions & 30 deletions lib/src/component_declaration/flux_component.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,17 @@
// Copyright 2016 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.

library over_react.component_declaration.flux_component;

import 'dart:async';
Expand All @@ -21,13 +35,13 @@ abstract class FluxUiProps<ActionsT, StoresT> extends UiProps {
ActionsT get actions => props[_actionsPropKey] as ActionsT; // ignore: avoid_as
set actions(ActionsT value) => props[_actionsPropKey] = value;

/// The prop defined by [StoresT]. This object should either be an
/// instance of [Store] or should provide access to one or more [Store]s.
/// The prop defined by [StoresT].
///
/// This object should either be an instance of [Store] or should provide access to one or more [Store]s.
///
/// **Instead of storing state within this component via [setState], it is
/// recommended that data be pulled directly from these stores.** This ensures
/// that the data being used is always up to date and leaves the state
/// management logic to the stores.
/// __Instead of storing state within this component via `setState`, it is recommended that data be
/// pulled directly from these stores.__ This ensures that the data being used is always up to date
/// and leaves the state management logic to the stores.
///
/// If this component only needs data from a single [Store], then [StoresT]
/// should be an instance of [Store]. This allows the default implementation
Expand All @@ -36,28 +50,30 @@ abstract class FluxUiProps<ActionsT, StoresT> extends UiProps {
/// If this component needs data from multiple [Store] instances, then
/// [StoresT] should be a class that provides access to these multiple stores.
/// Then, you can explicitly select the [Store] instances that should be
/// listened to by overriding [redrawOn].
/// listened to by overriding [_FluxComponentMixin.redrawOn].
StoresT get store => props[_storePropKey] as StoresT; // ignore: avoid_as
set store(StoresT value) => props[_storePropKey] = value;
}

/// Builds on top of [UiComponent], adding w_flux integration, much like the [FluxComponent] in w_flux.
///
/// Flux components are responsible for rendering application views and turning
/// user interactions and events into [Action]s. Flux components can use data
/// from one or many [Store] instances to define the resulting component.
/// * Flux components are responsible for rendering application views and turning
/// user interactions and events into [Action]s.
/// * Flux components can use data from one or many [Store] instances to define
/// the resulting component.
///
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
abstract class FluxUiComponent<TProps extends FluxUiProps> extends UiComponent<TProps>
with _FluxComponentMixin<TProps>, BatchedRedraws {}

/// Builds on top of [StatefulUiComponent], adding w_flux integration, much like the [FluxComponent] in w_flux.
/// Builds on top of [UiStatefulComponent], adding `w_flux` integration, much like the [FluxComponent] in w_flux.
///
/// Flux components are responsible for rendering application views and turning
/// user interactions and events into [Action]s. Flux components can use data
/// from one or many [Store] instances to define the resulting component.
/// * Flux components are responsible for rendering application views and turning
/// user interactions and events into [Action]s.
/// * Flux components can use data from one or many [Store] instances to define
/// the resulting component.
///
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
abstract class FluxUiStatefulComponent<TProps extends FluxUiProps, TState extends UiState>
extends UiStatefulComponent<TProps, TState>
with _FluxComponentMixin<TProps>, BatchedRedraws {}
Expand All @@ -68,15 +84,19 @@ abstract class FluxUiStatefulComponent<TProps extends FluxUiProps, TState extend
abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements BatchedRedraws {
TProps get props;

/// List of store subscriptions created when the component mounts. These
/// subscriptions are canceled when the component is unmounted.
/// List of store subscriptions created when the component mounts.
///
/// These subscriptions are canceled when the component is unmounted.
List<StreamSubscription> _subscriptions = [];

componentWillMount() {
// Subscribe to all applicable stores. Stores returned by `redrawOn()` will
// have their triggers mapped directly to this components redraw function.
// Stores included in the `getStoreHandlers()` result will be listened to
// and wired up to their respective handlers.
void componentWillMount() {
/// Subscribe to all applicable stores.
///
/// [Store]s returned by [redrawOn] will have their triggers mapped directly to this components
/// redraw function.
///
/// [Store]s included in the [getStoreHandlers] result will be listened to and wired up to their
/// respective handlers.
Map<Store, StoreHandler> handlers = new Map.fromIterable(redrawOn(),
value: (_) => (_) => redraw())..addAll(getStoreHandlers());

Expand All @@ -86,8 +106,8 @@ abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements Batche
});
}

componentWillUnmount() {
// ensure that unmounted components don't batch render
void componentWillUnmount() {
// Ensure that unmounted components don't batch render
shouldBatchRedraw = false;

// Cancel all store subscriptions.
Expand All @@ -99,6 +119,7 @@ abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements Batche
}

/// Define the list of [Store] instances that this component should listen to.
///
/// When any of the returned [Store]s update their state, this component will
/// redraw.
///
Expand All @@ -121,8 +142,9 @@ abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements Batche
}

/// If you need more fine-grained control over store trigger handling,
/// override this method to return a Map of stores to handlers. Whenever a
/// store in the returned map triggers, the respective handler will be called.
/// override this method to return a Map of stores to handlers.
///
/// Whenever a store in the returned map triggers, the respective handler will be called.
///
/// Handlers defined here take precedence over the [redrawOn] handling.
/// If possible, however, [redrawOn] should be used instead of this in order
Expand All @@ -132,9 +154,9 @@ abstract class _FluxComponentMixin<TProps extends FluxUiProps> implements Batche
return {};
}

/// Register a [subscription] that should be canceled when the component
/// unmounts. Cancellation will be handled automatically by
/// [componentWillUnmount].
/// Register a [subscription] that should be canceled when the component unmounts.
///
/// Cancellation will be handled automatically by [componentWillUnmount].
void addSubscription(StreamSubscription subscription) {
_subscriptions.add(subscription);
}
Expand Down
46 changes: 13 additions & 33 deletions lib/src/component_declaration/transformer_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ export './annotations.dart';
export './component_base.dart'
hide UiComponent, UiStatefulComponent, UiProps, UiState;

typedef dynamic _RefTypedef(String ref);

// ----------------------------------------------------------------------
// Helpers and extras consumable by generated code and consumers of
// generated code.
Expand Down Expand Up @@ -88,32 +86,22 @@ class GeneratedClass {
}


/// The basis for a over_react component, extending [react.Component]. (Successor to [BaseComponent]).
///
/// Includes support for strongly-typed props and utilities for prop and CSS classname forwarding.
/// See: [component_base.UiComponent]
///
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
abstract class UiComponent<TProps extends UiProps> extends component_base.UiComponent<TProps> with GeneratedClass {
/// This class should not be instantiated directly, and throws an error to indicate this.
UiComponent() {
_throwIfNotGenerated();
}

/// Returns the component of the specified [ref].
/// > `react.Component` if it is a Dart component
/// > DOM node if it is a DOM component.
///
/// Overridden for strong typing.
@override
_RefTypedef get ref => super.ref;

/// The default consumed props, taken from the keys generated in the associated @[Props] class.
/// The default consumed props, taken from the keys generated in the associated @[annotations.Props] class.
@toBeGenerated
Iterable<component_base.ConsumedProps> get $defaultConsumedProps => throw new UngeneratedError(member: #$defaultConsumedProps);

/// The keys for the non-forwarding props defined in this component.
///
/// For generated components, this defaults to the keys generated in the associated @[Props] class
/// For generated components, this defaults to the keys generated in the associated @[annotations.Props] class
/// if this getter is not overridden.
@override
Iterable<component_base.ConsumedProps> get consumedProps => $defaultConsumedProps;
Expand All @@ -126,55 +114,47 @@ abstract class UiComponent<TProps extends UiProps> extends component_base.UiComp
}


/// The basis for a stateful over_react component, extending [react.Component]. (Successor to [BaseComponentWithState]).
///
/// Includes support for strongly-typed props and state and utilities for prop and CSS classname forwarding.
/// See: [component_base.UiStatefulComponent]
///
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([annotations.Component]) annotation.
abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiState>
extends component_base.UiStatefulComponent<TProps, TState> with GeneratedClass {
/// This class should not be instantiated directly, and throws an error to indicate this.
UiStatefulComponent() {
_throwIfNotGenerated();
}

/// Returns the component of the specified [ref].
/// > `react.Component` if it is a Dart component
/// > DOM node if it is a DOM component.
///
/// Overridden for strong typing.
@override
_RefTypedef get ref => super.ref;

/// The default consumed prop keys, taken from the keys generated in the associated @[Props] class.
/// The default consumed prop keys, taken from the keys generated in the associated @[annotations.Props] class.
@toBeGenerated
Iterable<component_base.ConsumedProps> get $defaultConsumedProps => throw new UngeneratedError(member: #$defaultConsumedProps);

/// The keys for the non-forwarding props defined in this component.
///
/// For generated components, this defaults to the keys generated in the associated @[Props] class
/// For generated components, this defaults to the keys generated in the associated @[annotations.Props] class
/// if this getter is not overridden.
@override
Iterable<component_base.ConsumedProps> get consumedProps => $defaultConsumedProps;

/// Returns a typed props object backed by the specified [propsMap].
///
/// Required to properly instantiate the generic [TProps] class.
@override
@toBeGenerated
TProps typedPropsFactory(Map propsMap) => throw new UngeneratedError(member: #typedPropsFactory);

/// Returns a typed state object backed by the specified [stateMap].
///
/// Required to properly instantiate the generic [TState] class.
@override @toBeGenerated TState typedStateFactory(Map stateMap) => throw new UngeneratedError(member: #typedStateFactory);
}

/// A [dart.collection.MapView]-like class with strongly-typed getters/setters for React props that
/// is also capable of creating React component instances.
///
/// For use as a typed view into existing props [Maps], or as a builder to create new component
/// For use as a typed view into existing props [Map]s, or as a builder to create new component
/// instances via a fluent-style interface.
///
/// Use with the over_react transformer via the `@Props()` ([Props]) annotation.
/// Use with the over_react transformer via the `@Props()` ([annotations.Props]) annotation.
abstract class UiProps extends component_base.UiProps with GeneratedClass {
/// This class should not be instantiated directly, and throws an error to indicate this.
UiProps() {
Expand All @@ -189,7 +169,7 @@ abstract class UiProps extends component_base.UiProps with GeneratedClass {

/// A [dart.collection.MapView]-like class with strongly-typed getters/setters for React state.
///
/// Use with the over_react transformer via the `@State()` ([State]) annotation.
/// Use with the over_react transformer via the `@State()` ([annotations.State]) annotation.
abstract class UiState extends component_base.UiState with GeneratedClass {
/// This class should not be instantiated directly, and throws an error to indicate this.
UiState() {
Expand Down
16 changes: 0 additions & 16 deletions lib/src/util/document_event_helper_util.dart

This file was deleted.

14 changes: 0 additions & 14 deletions lib/src/util/js_util.dart

This file was deleted.

Loading

0 comments on commit 8fb0ba0

Please sign in to comment.