Issue #783 #948

IanCStewart · 2017-11-07T15:36:34Z

Merge Issue #930 #934 first

IanCStewart · 2017-11-07T15:48:41Z

Fix tests

IanCStewart · 2017-11-08T08:36:27Z

Fix select positioning

sjaakluthart · 2017-11-08T08:58:01Z

src/portal/index.jsx

+/* global document */
+import React from 'react';
+import PropTypes from 'prop-types';
+import { createPortal } from 'react-dom'; // eslint-disable-line import/no-extraneous-dependencies


You've made react-dome a dependency by importing this here, please install it as such and remove this eslint disable

sjaakluthart · 2017-11-08T08:59:02Z

src/portal/index.jsx

+class Portal extends React.Component {
+  static displayName = 'Portal'
+
+  static propTypes = {


Please define displayName, propTypes and defaultProps outside the Component

sjaakluthart · 2017-11-08T08:59:41Z

src/portal/index.jsx

+import isFunction from 'lodash/isFunction';
+
+/** For transportation of elements to document.body or an element of choice */
+class Portal extends React.Component {


Please import Component like this

import React, { Component } from 'react';

sjaakluthart · 2017-11-08T09:01:37Z

src/portal/index.jsx

+    /** Content to be portaled */
+    children: PropTypes.node.isRequired,
+    /** Optional node to portal children to */
+    node: PropTypes.instanceOf(Object)


Why would you want this to be optional? It makes more sense to always render it on the bottom of the body tag for example. Also I'm not sure if this default even works since you pass null as default prop...

If there is no node passed it appends to the bottom of the body. If a user want to append it to say a specific element within his app (say the main tag) it can pass this.

sjaakluthart · 2017-11-08T09:03:39Z

src/select/index.jsx

@@ -46,6 +46,8 @@ class Select extends Component {
    errorStyle: PropTypes.instanceOf(Object),
    /** The header's icon color */
    iconColor: PropTypes.string,
+    /** Node to portal PopOver to */
+    portalNode: PropTypes.instanceOf(Object),


Same thing goes for this portal prop, what's the use of this?

larstadema · 2017-11-08T09:02:28Z

src/portal/index.jsx

+    const { children, node } = this.props;
+
+    if (
+      (!isObject(window) && !window.document && !window.document.createElement)


Why not this: has(window, 'document.createElement')

larstadema · 2017-11-08T09:04:02Z

src/portal/index.jsx

+    /** Content to be portaled */
+    children: PropTypes.node.isRequired,
+    /** Optional node to portal children to */
+    node: PropTypes.instanceOf(Object)


Why is this instanceOf instead of node?

node breaks when u forward say document.getElementById('root'). This turns out to be a object not a node

larstadema · 2017-11-08T09:04:48Z

src/portal/index.jsx

+import isFunction from 'lodash/isFunction';
+
+/** For transportation of elements to document.body or an element of choice */
+class Portal extends React.Component {


Deconstruct Component in the import like we always do: import React, { Component } from 'react';

larstadema · 2017-11-08T09:05:52Z

src/portal/index.jsx

@@ -0,0 +1,53 @@
+/* global document */


Global document should be added to eslintrc like this:

"globals": { "document": false, "global": false, "navigator": false },

I meant to request changes

sjaakluthart · 2017-11-08T16:36:37Z

docs/src/components/select.jsx

@@ -1,3 +1,4 @@
+/* global document */


Please remove this comment, document isn't used in this file

sjaakluthart · 2017-11-08T16:39:47Z

src/portal/index.jsx

+  render() {
+    const { children, node } = this.props;
+
+    if (


Shouldn't you return children if document doesn't have createElement or createPortal doesn't exist? I'm not sure if your code works like this. Also you can just state has(document, 'createElement'), no need to check in window

fixed

* Issue #848 (#888) * Removed redundant typing message. * Removed redundant import. * Moved iconMenu into message body. * WIP added style to iconMenu for clean messages. * Moved sticker header on top of sticker body. * Added styles for sticker IconMenu. * Moved time on top of images and giphies. * Added styles for compact with new IconMenu placement. * Fixed collapsed styles. Fixes #848. * Fixed tests. * Added test for sticker messages. * Added styles for iconMenu in MessageHeader. * Bumped maxWidth of container to 80%. * Added readability maxWidth to TextMessage. * Issue #806 (#893) * Added menuIcon to Message. * Removed unused styles. * Replaced custom menus with iconMenu prop. * Added styles for iconMenu. * Applied API changes. * Fixed collapsedText. * Adjusted styles to look more like design. * Fixed sticker message styling. * Fixed tests. * Fixed sticker stretching. * Added font scaling for messageTime. * Removed redundant width styling from messageTime. * Fixed tests. * Issue #930 (#934) * Bumped react react-dom and html-react-parser version. * Bumped react and react-dom version. * Fixed all the tests. Fixes #930. * Created default style file to make message styles more dry. Fixes #494. (#949) * Replace dangerouslySetInnerHTML with html-react-parser. Fixes #761. (#951) * Replace dangerouslySetInnerHTML with html-react-parser. Fixes #761. * Removed createMarkup function. * Implemented 8dp grid to AppHeader. (#956) * Issue #751 alert (#955) * Implemented 8dp grid to Alert. * Cleaned-up Alert getStyle. * Reversed test. * Refactored message-list, made ref work again (#961) * Refactored message-list * renamed * Eslint fixes * Replaced emojione.imagePathPNG for emojione.emojiSize. Fixes #725. (#964) * Issue #783 (#948) * Bumped react react-dom and html-react-parser version. * Bumped react and react-dom version. * Fixed all the tests. Fixes #930. * Added react-portal to portal pop-over element to bottom of root. * Added own portal component. * Deconstructed props. * Added doc page about portal component. Fixes #783. * Added example for portal component. * Added description for docs page. * Added description for docs page. * Added alert about react v16. * Made passthrough for portal node prop instead. * Fixed Selects positioning. * Fixed pop-over-position test. * DocGen. * Adjusted in favor of comments. * Fixed broken check. * Changed example. * Changed text. * Cleanup. * Docgen. * Issue #872 (#950) * Added variable middle alignment for when it gets cut of. * Fixed horizontal middle so it actually alligns in the middle. * Added variable middle alignment for when it gets cut off horizontal. * Added 16 margin around the variable placements. * Fixed broken tests. * Added test for offset positioning. Fixes #872. * Removed logging. * Adjusted naming. * Implemented 8dp grid to Banner. (#968) * Implemented 8dp grid to Button. (#969) * Implemented 8dp grid to Card. (#970) * Issue #751 admin badge (#990) * Implemented 8dp grid to AdminBadge. * Fixed linting error. * Fixed broken test. * Implemented 8dp grid to AdminBadge. (#992)

IanCStewart added 13 commits November 2, 2017 14:17

Bumped react react-dom and html-react-parser version.

56e667c

Bumped react and react-dom version.

5fb1761

Fixed all the tests. Fixes #930.

745a649

Merge branch 'v5.0.0' into issue-#930

dfc238c

Added react-portal to portal pop-over element to bottom of root.

604d227

Added own portal component.

d1dde86

Deconstructed props.

4f16999

Merge branch 'v5.0.0' into issue-#783

e229d2d

Added doc page about portal component. Fixes #783.

0a59bdb

Added example for portal component.

031eba6

Added description for docs page.

934c6e5

Added description for docs page.

98dc8f7

Added alert about react v16.

5232a41

IanCStewart requested a review from sjaakluthart November 7, 2017 15:36

Made passthrough for portal node prop instead.

ba9869c

IanCStewart added 3 commits November 8, 2017 09:39

Fixed Selects positioning.

48690c0

Fixed pop-over-position test.

47fffc4

DocGen.

fafbb62

sjaakluthart suggested changes Nov 8, 2017

View reviewed changes

sjaakluthart previously approved these changes Nov 8, 2017

View reviewed changes

larstadema previously requested changes Nov 8, 2017

View reviewed changes

IanCStewart added 2 commits November 8, 2017 10:55

Adjusted in favor of comments.

c99f8ed

Merge branch 'v5.0.0' into issue-#783

948b30b

sjaakluthart previously requested changes Nov 8, 2017

View reviewed changes

Fixed broken check.

a19be8a

Sjaak Luthart added 3 commits November 13, 2017 12:22

Changed example.

cd1433e

Changed text.

27614e7

Cleanup.

8d78b6b

sjaakluthart approved these changes Nov 13, 2017

View reviewed changes

Docgen.

016069b

larstadema approved these changes Nov 13, 2017

View reviewed changes

larstadema merged commit 91cd977 into v5.0.0 Nov 13, 2017

larstadema deleted the issue-#783 branch November 13, 2017 12:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #783 #948

Issue #783 #948

IanCStewart commented Nov 7, 2017 •

edited by sjaakluthart

Loading

IanCStewart commented Nov 7, 2017

IanCStewart commented Nov 8, 2017 •

edited

Loading

sjaakluthart Nov 8, 2017

sjaakluthart Nov 8, 2017

sjaakluthart Nov 8, 2017

sjaakluthart Nov 8, 2017 •

edited

Loading

IanCStewart Nov 8, 2017

sjaakluthart Nov 8, 2017 •

edited

Loading

larstadema Nov 8, 2017

larstadema Nov 8, 2017

IanCStewart Nov 8, 2017

larstadema Nov 8, 2017

larstadema Nov 8, 2017

sjaakluthart Nov 8, 2017

sjaakluthart Nov 8, 2017

Issue #783 #948

Issue #783 #948

Conversation

IanCStewart commented Nov 7, 2017 • edited by sjaakluthart Loading

IanCStewart commented Nov 7, 2017

IanCStewart commented Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjaakluthart Nov 8, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sjaakluthart Nov 8, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IanCStewart commented Nov 7, 2017 •

edited by sjaakluthart

Loading

IanCStewart commented Nov 8, 2017 •

edited

Loading

sjaakluthart Nov 8, 2017 •

edited

Loading

sjaakluthart Nov 8, 2017 •

edited

Loading