-
Notifications
You must be signed in to change notification settings - Fork 30
Initial commit of component render classes. #8
Changes from 4 commits
0638019
e5d96a8
8500f7e
dad1c8b
3c4c4fe
2a0e894
6bc9a3e
db0fa11
939dbf7
f5d12f7
9ff18a0
2840c31
9bde50b
be4a5d8
bef5a8e
5d548ad
f791971
95e2423
fffa746
e1ea4fc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,175 @@ | ||
/** @license | ||
* Copyright 2016 Google Inc. All Rights Reserved. | ||
* | ||
* 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. | ||
*/ | ||
|
||
import "./overlay/styles.less"; | ||
import * as React from "react"; | ||
import * as ReactDOM from "react-dom"; | ||
import { Constants as CONST } from "../lib/Constants"; | ||
import { Messaging } from "../lib/Messaging"; | ||
import { Options } from "./overlay/Options"; | ||
import { Overlay } from "./overlay/Overlay"; | ||
import { Variable } from "../core/variables/Variable"; | ||
import { remixer } from "../core/Remixer"; | ||
|
||
/** | ||
* Interface for the properties assigned to the DOMController component. | ||
* @interface | ||
*/ | ||
interface ControllerProps { | ||
wrapperElement: HTMLElement; | ||
} | ||
|
||
/** | ||
* Interface for the state of the DOMController component. | ||
* @interface | ||
*/ | ||
interface ControllerState { | ||
/** | ||
* The DOMController will render a control component for each of the variables | ||
* of this array. | ||
* @type {Array<Variable>} | ||
*/ | ||
variables?: Array<Variable>; | ||
|
||
/** | ||
* The current route to render. | ||
* @type {Route} | ||
*/ | ||
route?: Route; | ||
} | ||
|
||
/** | ||
* Available routes to render. | ||
*/ | ||
enum Route { | ||
Variables, | ||
Options | ||
} | ||
|
||
/** | ||
* A React component class that renders an MDL-styled card containing child | ||
* components determined by its current Route. | ||
* | ||
* Depending on the Route, the card content can be either: | ||
* 1) The overlay with configurable control components per assigned Variables. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The language could be simpler: "Controls to tweak the values for each Variable" This comment seems like it would fit the |
||
* 2) An options page used to configure Remixer session properties. | ||
* | ||
* @class | ||
* @extends React.Component | ||
*/ | ||
export class DOMController extends React.Component<ControllerProps, ControllerState> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This component it trying to do too many things. See my advice in #12. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks! After we land this I will do some refactoring per #12. |
||
|
||
/** | ||
* Default constructor. | ||
* @constructor | ||
* @type {DOMController} | ||
*/ | ||
constructor(props: ControllerProps) { | ||
super(props); | ||
this.state = {variables: remixer.attachedInstance.variablesArray, route: Route.Variables}; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think in TypeScript you can set There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure I'm following your suggestion here. I'm setting state initially in the constructor based on guidelines here: https://facebook.github.io/react/docs/state-and-lifecycle.html. Can you clarify what your request is here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. class Thing extends React.Component {
state = {
something: 123,
}
// Now you don't need a custom constructor
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
} | ||
|
||
/** | ||
* The component will mount. Lets register for messaging and key events. | ||
* @override | ||
*/ | ||
componentDidMount() { | ||
Messaging.register(this.onMessageReceived.bind(this)); | ||
this.addKeyListener(); | ||
} | ||
|
||
/** | ||
* The component will unmount. Lets unregister. | ||
* @override | ||
*/ | ||
componentWillUnmount() { | ||
Messaging.unregister(); | ||
this.removeKeyListener(); | ||
} | ||
|
||
/** | ||
* Handles receiving of window message. | ||
* @param {MessageEvent} event The received message event. | ||
*/ | ||
onMessageReceived(event: MessageEvent): void { | ||
if (event.data === Messaging.type.ToggleVisibility) { | ||
this.toggleVisibility(); | ||
} | ||
} | ||
|
||
/** Adds a key listener. */ | ||
addKeyListener(): void { | ||
document.addEventListener(CONST.KEY_EVENT_DOWN, (e: KeyboardEvent) => { | ||
if (e.keyCode === CONST.KEY_CODE_ESC) { | ||
this.toggleVisibility(); | ||
} | ||
}); | ||
} | ||
|
||
/** Removes a key listener. */ | ||
removeKeyListener(): void { | ||
document.removeEventListener(CONST.KEY_EVENT_DOWN); | ||
} | ||
|
||
/** Toggles the Remixer overlay visibility. */ | ||
toggleVisibility() { | ||
this.props.wrapperElement.classList.toggle(CONST.CSS_CLASS_VISIBLE); | ||
} | ||
|
||
/** | ||
* Updates the state to new route. | ||
* @param {Event} event The route to update to. | ||
*/ | ||
updateRoute(event: Event) { | ||
const currentRoute = this.state.route; | ||
this.setState({ | ||
route: currentRoute === Route.Variables | ||
? Route.Options | ||
: Route.Variables | ||
}); | ||
} | ||
|
||
/** | ||
* Renders the component via React. | ||
* @override | ||
*/ | ||
render() { | ||
const CurrentRoute = this.state.route === Route.Options ? Options : Overlay; | ||
return ( | ||
<div className="rmx-card-wide mdl-card mdl-shadow--6dp"> | ||
<div className="mdl-card__title" ref="myInput"> | ||
<h2 className="mdl-card__title-text">Remixer</h2> | ||
|
||
</div> | ||
<div className="mdl-card__menu"> | ||
<button className="mdl-button mdl-button--icon mdl-js-button mdl-js-ripple-effect" | ||
onClick={this.updateRoute.bind(this)}> | ||
<i className="material-icons">menu</i> | ||
</button> | ||
</div> | ||
<div className="mdl-card__supporting-text mdl-card__actions mdl-card--border"> | ||
<CurrentRoute variables={this.state.variables} /> | ||
</div> | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* Renders the DOMController component to the overlay wrapper element. | ||
*/ | ||
let element = document.getElementById(CONST.ID_OVERLAY_WRAPPER); | ||
ReactDOM.render(<div><DOMController wrapperElement={element} /></div>, element); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,86 @@ | ||
/** @license | ||
* Copyright 2016 Google Inc. All Rights Reserved. | ||
* | ||
* 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. | ||
*/ | ||
|
||
import * as React from "react"; | ||
import { Component } from "./Component"; | ||
|
||
interface ColorSwatchProps { | ||
color: string; | ||
isSelected: boolean; | ||
onClick: any; | ||
} | ||
|
||
/** | ||
* A React component class that renders a single color swatch. | ||
* @class | ||
* @extends React.Component | ||
*/ | ||
class ColorSwatch extends React.Component<ColorSwatchProps, void> { | ||
render() { | ||
let { color, isSelected } = this.props; | ||
return ( | ||
<div className="rmx-color-item" style={{backgroundColor: color}} | ||
value={color} onClick={this.props.onClick}> | ||
{ isSelected ? <i className="material-icons">check</i> : "" } | ||
</div> | ||
); | ||
} | ||
} | ||
|
||
/** | ||
* A React component class that renders a color swatch picker component. | ||
* @class | ||
* @extends Component | ||
*/ | ||
export class ColorSwatchComponent extends Component { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You typed your props in the Wait, I see now that you're extending a custom |
||
|
||
/** @override */ | ||
static numberOfLines: number = 2; | ||
|
||
/** @override */ | ||
get id(): string { | ||
return `rmx-swatch-${this.props.variable.key}`; | ||
} | ||
|
||
/** | ||
* Handles updating the selected color when swatch is selected. | ||
* @param {string} value The new selected color value. | ||
*/ | ||
updateSelectedColor(color: string) { | ||
this.updateSelectedValue(color); | ||
} | ||
|
||
/** @override */ | ||
render() { | ||
let possibleValues: Array<string> = this.props.variable["possibleValues"]; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Dictionary notation is frowned on in Google Style. This could be |
||
let { selectedValue } = this.state; | ||
|
||
// Ensure selected value is choosable from possible values. | ||
if (possibleValues.indexOf(selectedValue) === -1) { | ||
possibleValues.push(selectedValue); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're modifying the props you get passed in? That makes it hard for the rest of the code to reason about Can you break this out into its own method and call it when storeValuesInState() {
const {
possibleValues,
selectedValue,
} = this.state;
if (!possibleValues.includes(selectedValue)) {
this.setState(
{
possibleValues: [...possibleValues, selectedValue],
}
);
}
} |
||
}; | ||
|
||
return ( | ||
<div> | ||
{possibleValues.map((value: string, i: number) => ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be a lot easier to read if you had a <ColoredCircle
radius = { 56 }
color = { value }
onClick = { this.onClick }
>
{
selectedValue === value
? <MaterialIcon>
check
</MaterialIcon>
: null
}
</ColoredCircle> There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. Added new |
||
<ColorSwatch color={value} key={value} | ||
isSelected={selectedValue === value} | ||
onClick={this.updateSelectedColor.bind(this, value)} /> | ||
))} | ||
</div> | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,87 @@ | ||
/** @license | ||
* Copyright 2016 Google Inc. All Rights Reserved. | ||
* | ||
* 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. | ||
*/ | ||
|
||
import * as React from "react"; | ||
import { ComponentFactory, ComponentProps } from "./ComponentFactory"; | ||
|
||
/** | ||
* Interface for a Component class. | ||
* @interface | ||
*/ | ||
interface ComponentType { | ||
id: string; | ||
updateSelectedValue(newValue: any): void; | ||
} | ||
|
||
/** | ||
* Interface for the state of a Component class. | ||
* @interface | ||
*/ | ||
interface ComponentState { | ||
selectedValue: any; | ||
} | ||
|
||
/** | ||
* A React component class that Remixer control components should subclass. | ||
* @class | ||
* @extends React.Component | ||
* @implements Component | ||
*/ | ||
export class Component extends React.Component<ComponentProps, ComponentState> implements ComponentType { | ||
constructor(props: ComponentProps) { | ||
super(props); | ||
this.state = {selectedValue: props.variable.selectedValue}; | ||
} | ||
|
||
/** | ||
* Number of lines to render this component. Defaults to 1. | ||
* @override | ||
* @static | ||
* @type {number} | ||
*/ | ||
static numberOfLines: number = 1; | ||
|
||
/** | ||
* The HTML id for this component. Subclasses should override and provide a | ||
* unique id in the form of `rmx-{component-name}-key`. | ||
* | ||
* As an example: rmx-switch-title. | ||
* | ||
* @readonly. | ||
* @return {string} The HTML element ID. | ||
*/ | ||
id: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How is this used? As I noted in another comment, reading values off of a React component feels strange. |
||
|
||
/** | ||
* The component will mount. Dynamicly generate MDL elements require | ||
* calling `upgradeElement` for each component. | ||
* @override | ||
*/ | ||
componentDidMount() { | ||
for (let key in this.refs) { | ||
window["componentHandler"].upgradeElement(this.refs[key]); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See my note about dictionary access. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately in this instance, componentHandler is added from the MDL library to window dynamically, and typescript complains if accessing this in another way besides dict. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If you're using TypeScript, you should probably type these references. I think it's interface ComponentHandler {
upgradeElement: (element:Element) => void;
}
declare componentHandler: ComponentHandler; Maybe @traviskaufman already has type declarations you can use. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
} | ||
} | ||
|
||
/** | ||
* Updates the state and selected value. | ||
* @param {any} newValue The new value to update to. | ||
*/ | ||
updateSelectedValue(newValue: any) { | ||
this.setState({selectedValue: newValue}); | ||
this.props.variable.selectedValue = newValue; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why
import * as React from 'react
as opposed to justimport React from 'react'
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately React does not have a default export specified, and therefore your suggestion won't work in typescript. I'm importing as per Typescript documentation on working with React here: https://www.typescriptlang.org/docs/handbook/react-&-webpack.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know! I've always used Babel, which converts CommonJS modules to use default exports automatically.