Skip to content

Commit

Permalink
feat(InfoWindow): add ReactElement child as content support
Browse files Browse the repository at this point in the history 
* Also add this support to InfoBox as well
* Only one child is supported inside <InfoWindow> or <InfoBox>
* Closes #69 and thanks to @thetiby
  • Loading branch information
@tomchentw
tomchentw committed Aug 7, 2015
1 parent 4511d87 commit 2c0dc02
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 23 deletions.
25 changes: 16 additions & 9 deletions examples/gh-pages/scripts/components/basics/StyledMap.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,16 @@ export default class StyledMap extends Component {
}]
}

_click_from_children_of_infoBox = (e) => {
console.log("_click_from_children_of_infoBox!!");
console.log(e);
}

render () {
const {props, state} = this,
{googleMapsApi, mapStyles, ...otherProps} = props;
const myLatLng = new google.maps.LatLng(25.03, 121.6);

const InfoBoxContent = `
<div style="background-color:yellow; opacity:0.75;">
<div style="font-size: 16px; font-color:#08233B">
Taipei
</div>
</div>
`;
return (
<GoogleMap containerProps={{
...otherProps,
Expand All @@ -126,9 +124,18 @@ export default class StyledMap extends Component {
defaultZoom={5}
defaultCenter={myLatLng}>
<InfoBox
closeBoxURL=""
defaultPosition={myLatLng}
defaultContent={InfoBoxContent}/>
options={{closeBoxURL: "", enableEventPropagation: true}}
>
<div
style={{backgroundColor: "yellow", opacity: 0.75}}
onClick={this._click_from_children_of_infoBox}
>
<div style={{fontSize: "16px", fontColor: "#08233B"}}>
Taipei
</div>
</div>
</InfoBox>
</GoogleMap>
);
}
Expand Down
34 changes: 26 additions & 8 deletions examples/gh-pages/scripts/components/events/ClosureListeners.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,31 @@ export default class ClosureListeners extends Component {
this.setState(this.state);
}

_render_InfoWindow (ref, marker) {
if (Math.random() > 0.5) {
// Normal version: Pass string as content
return (
<InfoWindow key={`${ref}_info_window`}
content={marker.content}
onCloseclick={this._handle_closeclick.bind(this, marker)}
/>
)
} else {
// "react-google-maps" extended version: Pass ReactElement as content
return (
<InfoWindow key={`${ref}_info_window`}
onCloseclick={this._handle_closeclick.bind(this, marker)}
>
<div>
<strong>{marker.content}</strong>
<br />
<em>The contents of this InfoWindow are actually ReactElements.</em>
</div>
</InfoWindow>
)
}
}

render () {
const {markers} = this.state;

Expand All @@ -65,19 +90,12 @@ export default class ClosureListeners extends Component {
{markers.map((marker, index) => {
const ref = `marker_${index}`;

const content = marker.showInfo ? (
<InfoWindow key={`${ref}_info_window`}
content={marker.content}
onCloseclick={this._handle_closeclick.bind(this, marker)}
/>
) : null;

return (
<Marker key={ref} ref={ref}
position={marker.position}
title={(index+1).toString()}
onClick={this._handle_marker_click.bind(this, marker)}>
{content}
{marker.showInfo ? this._render_InfoWindow(ref, marker) : null}
</Marker>
);
})}
Expand Down
13 changes: 10 additions & 3 deletions src/addons/addonsCreators/InfoBoxCreator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import {default as InfoBoxEventList} from "../addonsEventLists/InfoBoxEventList"
import {default as eventHandlerCreator} from "../../utils/eventHandlerCreator";
import {default as defaultPropsCreator} from "../../utils/defaultPropsCreator";
import {default as composeOptions} from "../../utils/composeOptions";
import {default as setContentForOptionalReactElement} from "../../utils/setContentForOptionalReactElement";
import {default as componentLifecycleDecorator} from "../../utils/componentLifecycleDecorator";

import {default as GoogleMapHolder} from "../../creators/GoogleMapHolder";

export const infoBoxControlledPropTypes = {
// http://google-maps-utility-library-v3.googlecode.com/svn/trunk/infobox/docs/reference.html
content: PropTypes.any, /* TODO: children */
content: PropTypes.any,
options: PropTypes.object,
position: PropTypes.any,
visible: PropTypes.bool,
Expand All @@ -24,7 +25,8 @@ export const infoBoxControlledPropTypes = {
export const infoBoxDefaultPropTypes = defaultPropsCreator(infoBoxControlledPropTypes);

const infoBoxUpdaters = {
content (/* content, component */) { /* TODO: children */ },
children (children, component) { setContentForOptionalReactElement(children, component.getInfoWindow()); },
content (content, component) { component.getInfoBox().setContent(content); },
options (options, component) { component.getInfoBox().setOptions(options); },
position (position, component) { component.getInfoBox().setPosition(position); },
visible (visible, component) { component.getInfoBox().setVisible(visible); },
Expand Down Expand Up @@ -56,11 +58,16 @@ export default class InfoBoxCreator extends Component {
// http://google-maps-utility-library-v3.googlecode.com/svn/trunk/infobox/docs/reference.html
const infoBox = new GoogleMapsInfobox(composeOptions(infoBoxProps, [
// https://developers.google.com/maps/documentation/javascript/3.exp/reference
"content", /* TODO: children */
"content",
"position",
"visible",
"zIndex",
]));

if (infoBoxProps.children) {
setContentForOptionalReactElement(infoBoxProps.children, infoBox);
}

if (anchorHolderRef) {
infoBox.open(mapHolderRef.getMap(), anchorHolderRef.getAnchor());
} else {
Expand Down
12 changes: 9 additions & 3 deletions src/creators/InfoWindowCreator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import {default as InfoWindowEventList} from "../eventLists/InfoWindowEventList"
import {default as eventHandlerCreator} from "../utils/eventHandlerCreator";
import {default as defaultPropsCreator} from "../utils/defaultPropsCreator";
import {default as composeOptions} from "../utils/composeOptions";
import {default as setContentForOptionalReactElement} from "../utils/setContentForOptionalReactElement";
import {default as componentLifecycleDecorator} from "../utils/componentLifecycleDecorator";

import {default as GoogleMapHolder} from "./GoogleMapHolder";

export const infoWindowControlledPropTypes = {
// [].map.call($0.querySelectorAll("tr>td>code"), function(it){ return it.textContent; }).filter(function(it){ return it.match(/^set/) && !it.match(/^setMap/); })
// https://developers.google.com/maps/documentation/javascript/3.exp/reference#InfoWindow
content: PropTypes.any, /* TODO: children */
content: PropTypes.any,
options: PropTypes.object,
position: PropTypes.any,
zIndex: PropTypes.number,
Expand All @@ -24,7 +25,8 @@ export const infoWindowControlledPropTypes = {
export const infoWindowDefaultPropTypes = defaultPropsCreator(infoWindowControlledPropTypes);

const infoWindowUpdaters = {
content (/* content, component */) { /* TODO: children */ },
children (children, component) { setContentForOptionalReactElement(children, component.getInfoWindow()); },
content (content, component) { component.getInfoWindow().setContent(content); },
options (options, component) { component.getInfoWindow().setOptions(options); },
position (position, component) { component.getInfoWindow().setPosition(position); },
zIndex (zIndex, component) { component.getInfoWindow().setZIndex(zIndex); },
Expand All @@ -51,11 +53,15 @@ export default class InfoWindowCreator extends Component {
// https://developers.google.com/maps/documentation/javascript/3.exp/reference#InfoWindow
const infoWindow = new google.maps.InfoWindow(composeOptions(infoWindowProps, [
// https://developers.google.com/maps/documentation/javascript/3.exp/reference#InfoWindowOptions
"content", /* TODO: children */
"content",
"position",
"zIndex",
]));

if (infoWindowProps.children) {
setContentForOptionalReactElement(infoWindowProps.children, infoWindow);
}

if (anchorHolderRef) {
infoWindow.open(mapHolderRef.getMap(), anchorHolderRef.getAnchor());
} else {
Expand Down
32 changes: 32 additions & 0 deletions src/utils/setContentForOptionalReactElement.js
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import {
default as React,
Children,
} from "react";

function renderElement (
contentElement,
prevContent
) {
if ("[object HTMLDivElement]" !== Object.prototype.toString.call(prevContent)) {
prevContent = document.createElement("div");
}

React.render(contentElement, prevContent);
return prevContent;
}

export default function setContentForOptionalReactElement (
contentOptionalReactElement,
infoWindowLikeInstance
) {
if (React.isValidElement(contentOptionalReactElement)) {
const contentElement = Children.only(contentOptionalReactElement);
const prevContent = infoWindowLikeInstance.getContent();

const domEl = renderElement(contentOptionalReactElement, prevContent);

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 7, 2015

@tomchentw thank you so much for including this functionality in the main repo!

Did you keep the functionality of setting a class name on the DOM element where the infoWindow/infoBox child is mounted? I love your new code structure, but I think it's important to also have this in. (because there most likely will be the need to style the parent element of whatever content I put into the InfoBox, given that the parent element can be an empty div with no css class to reference it by)

For reference, I used to do that here. Thanks!

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 7, 2015

Author Owner

Thanks for the review. Yes I did remove the functionality of setting className on the empty <div>. The reason is, I think that opens a hole for leaky of abstraction. I'm thinking that we should have a better way to do this.
Actually, I'm not good at CSS. Would you mind to provide an example/usecase that indicates a blank <div> that would affects the style on your InfoWindow?
Thanks!

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 7, 2015

@tomchentw Thanks for the quick reply!

Here, on this family vacation site I'm working on, if you click a pin on the map, the infoBox that show's up has a little 'i' icon. When you click that, you'll notice that the whole card flips over. Without the class name pin-card on the direct child of the InfoBox, I wouldn't be able to do this effect, because the unstyled div would be white (and not even positioned correctly).

I'm using my own fork on that site in order to be able to use your repo, for the time being, but it would be great if you could get this in!

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 7, 2015

Author Owner

I tried to manually add a <div> to wrap .pin-card and it seems to work okay:

screen shot 2015-08-07 at 10 45 08 pm

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 7, 2015

Yep, it does work, I must've did something wrong the first time I tried this and then I didn't try again. Still, by being able to set the class on the empty div, I was able to get a better DOM representation (without an extra, meaningless div) that's just required so that React can work correctly.

That's a lot of extra divs for my pin count :). Doesn't seem like a requirement, indeed, but it's a really nice to have in my opinion (actually, the ability to give any number of attributes to that div would be ideal).

What did you mean by:

opens a hole for leaky of abstraction

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 8, 2015

Author Owner

the ability to give any number of attributes to that div would be ideal)
This is what I means for leaky of abstraction.

I understand your concern/unconfortable of an extra empty div. But until people could find a better way of doing this, I would like to keep it unchanged. Sorry for any inconvenience :(

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 8, 2015

I understand what you meant by leaky abstraction - indeed, the library user shouldn't need to manually set attributes on that div.

How about this, then? Even though it's not in the public API description, the InfoBox div is heavily used in methods that are public and it is also public via InfoBoxInstance.div_. I know this might sound hacky, but I think that the InfoBox React child can be mounted directly to that div, eliminating the need of creating a new one. (I'm not sure if we can contribute to Google Code, especially that it's going to become read only very soon, but I think a getter like InfoBoxInstance.getDiv() to return infoBoxInstace.div_ would be great, hiding the implementation detail.)

What do you think?

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 8, 2015

Author Owner

I'm okay with that, the next question is, how to deal with stock InfoWindow?

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 8, 2015 •
edited
Loading

Hmm. Here's a way that I found:

  1. initialize the InfoWindow with a fake dom element (just like now) - wrote this directly in console to test it out so forgive the lack of elegance.
var d = document.createElement('div');
d.innerHTML="tiby";

infowindow = new google.maps.InfoWindow({
  content: d
});
  1. wait for that dom element to be appended to the google maps dom; once that happend, use it to figure out the dom element we actually want to mount our react component to
google.maps.event.addListener(infowindow, 'domready', function() {
 var currentContent = this.getContent(), parent = currentContent.parentNode;
console.log(currentContent); // will be the fake element the first time it shows up after which it will be our React content
if (!currentContent.isRenderedByReact) { 
   parent.innerHTML = ""; // to allow React to render smoothly
   var reactDOM = document.createElement('div'); // React.render(ReactChild, parent); 
   reactDOM.innerHTML = "this content is rendered by react"; // reactDOM = React.findDOMNode(...)
   parent.appendChild(reactDOM); // simulates that React has succesfully renderd to the parent
   reactDOM.isRenderedByReact = true;
   this.setContent(reactDOM); // now we link the actual InfoWindow instance to the React rendered element
};
});

I tested this directly on google's website; paused the code at line 54 (var marker = new google.marker [...]) and injected both above snippets. The fake 'tiby' div flashes briefly on the first opening of the InfoWindow, but this won't be the case here because that fake div will be empty for the lib.

What do you think?

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 8, 2015

Author Owner

I'm afraid this is too overkilled for a library. Plus this interacts with a brunch of implementation details (though it should be fine).

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 8, 2015

Well, I, for one, would suggest leaving InfoWindow alone. It can be done, but InfoWindow already has a lot of divs and it's not really flexible. However, it'd be really nice if you could include the usage of the "InfoBox" div as the React render context. (the instance.div_ actually comes from Google's OverlayView which is a superclass for InfoBox, so I doubt that's subject to change)

Thanks again for your quick responses!

This comment has been minimized.

Sign in to view

Copy link
@tomchentw

tomchentw Aug 8, 2015

Author Owner

Sounds great. And we could fix <OverlayView> as well. No more empty divs ... Could you open an issue for me? Thanks!

This comment has been minimized.

Sign in to view

Copy link
@thetiby

thetiby Aug 8, 2015

Here you go: #93
infoWindowLikeInstance.setContent(domEl);

} else {
infoWindowLikeInstance.setContent(contentOptionalReactElement);
}
}

0 comments on commit 2c0dc02

Please sign in to comment.