Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIP-1953 Sync changes from OverReact source #40

Merged
merged 3 commits into from
Jan 23, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -495,15 +495,15 @@ as shown in the examples above.

## Component Formatting
> __A note on dart_style:__
>
>
> Currently, [dart_style (dartfmt)](https://github.com/dart-lang/dart_style) decreases the readability of components
> built using [OverReact's fluent-style](#fluent-style-component-consumption).
> See https://github.com/dart-lang/dart_style/issues/549 for more info.
>
>
> We're exploring some different ideas to improve automated formatting, but for the time being, we __do not recommend__ using dart_style with OverReact.
>
>
> However, if you do choose to use dart_style, you can greatly improve its output by using trailing commas in children argument lists:
>
>
> * dart_style formatting:
> ```dart
> return (Button()
Expand Down
1 change: 1 addition & 0 deletions lib/over_react.dart
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export 'src/util/key_constants.dart';
export 'src/util/map_util.dart';
export 'src/util/pretty_print.dart';
export 'src/util/prop_errors.dart';
export 'src/util/prop_key_util.dart';
export 'src/util/react_wrappers.dart';
export 'src/util/rem_util.dart';
export 'src/util/string_util.dart';
Expand Down
9 changes: 9 additions & 0 deletions lib/src/component/callback_typedefs.dart
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@

library over_react.callback_typedefs;

import 'dart:html';

import 'package:over_react/over_react.dart' show ResizeSensorEvent;
import 'package:react/react.dart' as react;

// Callbacks for React's DOM event system
Expand All @@ -29,3 +32,9 @@ typedef WheelEventCallback(react.SyntheticWheelEvent event);

/// A generic callback that takes no arguments.
typedef Callback();

// Callback for DOM elements
typedef Element ElementCallback();

// Callback for [ResizeSensorEvent]s
typedef void ResizeSensorHandler(ResizeSensorEvent event);
18 changes: 9 additions & 9 deletions lib/src/component/resize_sensor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,12 @@
/// Thanks!
/// https://github.com/marcj/css-element-queries/blob/master/src/ResizeSensor.js
library resize_sensor;

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

import 'package:browser_detect/browser_detect.dart';
import 'package:over_react/over_react.dart';
import 'package:platform_detect/platform_detect.dart';
import 'package:react/react.dart' as react;

// Callback for [ResizeSensorEvent]s
typedef void ResizeSensorHandler(ResizeSensorEvent event);
import 'package:over_react/over_react.dart';

/// A wrapper component that detects when its parent is resized.
///
Expand Down Expand Up @@ -82,6 +78,7 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> {

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

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

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

var collapseSensor = (Dom.div()
..className = 'resize-sensor-collapse'
Expand Down Expand Up @@ -152,9 +152,9 @@ class ResizeSensorComponent extends UiComponent<ResizeSensorProps> {
};

// IE 10 and Safari 8 need 'special' value prefixes for 'display:flex'.
if (browser.isIe && browser.version <= '10') {
if (browser.isInternetExplorer && browser.version.major <= 10) {
wrapperStyles['display'] = '-ms-flexbox';
} else if (browser.isSafari && browser.version < '9') {
} else if (browser.isSafari && browser.version.major < 9) {
wrapperStyles['display'] = '-webkit-flex';
} else {
wrapperStyles['display'] = 'flex';
Expand Down
33 changes: 20 additions & 13 deletions lib/src/component_declaration/component_base.dart
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import 'package:over_react/over_react.dart' show
prettyPrintMap,
unindent,
PropError;

import 'package:over_react/src/component_declaration/component_type_checking.dart';
import 'package:react/react.dart' as react;
import 'package:react/react_client.dart';
Expand Down Expand Up @@ -85,14 +86,20 @@ 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>();

/// The basis for a over_react component.
typedef dynamic _RefTypedef(String ref);

/// 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.
///
/// 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought we removed these redeclaration of refs when we added the typing to react-dart.


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

Expand Down Expand Up @@ -124,12 +131,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);
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nit looks like this was fixed in over_react after the code diverged

});
}

Expand Down Expand Up @@ -186,6 +193,7 @@ abstract class UiComponent<TProps extends UiProps> extends react.Component {
TProps typedPropsFactory(Map propsMap);

/// Returns a typed props object backed by a new Map.
///
/// Convenient for use with [getDefaultProps].
TProps newProps() => typedPropsFactory({});

Expand All @@ -195,11 +203,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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nit looks like this was fixed in over_react after the code diverged

///
/// Includes support for strongly-typed props and state and utilities for prop and CSS classname forwarding.
///
/// Extends [react.Component].
/// Extends [react.Component]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#nit looks like this was fixed in over_react after the code diverged

///
/// Related: [UiComponent]
abstract class UiStatefulComponent<TProps extends UiProps, TState extends UiState> extends UiComponent<TProps> {
Expand Down Expand Up @@ -232,7 +240,6 @@ 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);

Expand Down
68 changes: 30 additions & 38 deletions lib/src/component_declaration/flux_component.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,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].
/// The prop defined by [StoresT]. This object should either be an
/// instance of [Store] or should provide access to one or more [Store]s.
///
/// 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,30 +36,28 @@ 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 [_FluxComponentMixin.redrawOn].
/// listened to by overriding [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()` ([annotations.Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
abstract class FluxUiComponent<TProps extends FluxUiProps> extends UiComponent<TProps>
with _FluxComponentMixin<TProps>, BatchedRedraws {}

/// Builds on top of [UiStatefulComponent], adding `w_flux` integration, much like the [FluxComponent] in w_flux.
/// Builds on top of [StatefulUiComponent], 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()` ([annotations.Component]) annotation.
/// Use with the over_react transformer via the `@Component()` ([Component]) annotation.
abstract class FluxUiStatefulComponent<TProps extends FluxUiProps, TState extends UiState>
extends UiStatefulComponent<TProps, TState>
with _FluxComponentMixin<TProps>, BatchedRedraws {}
Expand All @@ -70,19 +68,15 @@ 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 = [];

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.
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.
Map<Store, StoreHandler> handlers = new Map.fromIterable(redrawOn(),
value: (_) => (_) => redraw())..addAll(getStoreHandlers());

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

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

// Cancel all store subscriptions.
Expand All @@ -105,7 +99,6 @@ 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 @@ -128,9 +121,8 @@ 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 @@ -140,9 +132,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].
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the version of this file was more up to date in over_react (i.e., these changes should be discarded)

void addSubscription(StreamSubscription subscription) {
_subscriptions.add(subscription);
}
Expand Down
Loading