Tweaks to new Wrapperless mode #204
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,4 +2,4 @@ | |
| { | ||
| "javascript.validate.enable": false, | ||
| "eslint.enable": true | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,21 +24,6 @@ Flip Move uses the [_FLIP technique_](https://aerotwist.com/blog/flip-your-anima | |
| * <a href="http://joshwcomeau.github.io/react-flip-move/examples/#/scrabble" target="_blank">__Scrabble__</a> | ||
| * <a href="http://joshwcomeau.github.io/react-flip-move/examples/#/laboratory" target="_blank">__Laboratory__</a> | ||
|
|
||
|
|
||
|
|
||
| ## Installation | ||
|
|
@@ -114,45 +99,95 @@ Curious how this works, under the hood? [__Read the Medium post__](https://mediu | |
|
|
||
| --- | ||
|
|
||
| ### Wrapping Element | ||
|
|
||
| By default, FlipMove wraps the children you pass it in a `<div>`: | ||
|
|
||
| ```jsx | ||
| // JSX | ||
| <FlipMove> | ||
| <div key="a">Hello</div> | ||
| <div key="b">World</div> | ||
| </FlipMove> | ||
|
|
||
| // HTML | ||
| <div> | ||
| <div>Hello</div> | ||
| <div>World</div> | ||
| </div> | ||
| ``` | ||
|
|
||
| Any unrecognized props to `<FlipMove>` will be delegated to this wrapper element: | ||
|
|
||
| ```jsx | ||
| // JSX | ||
| <FlipMove className="flip-wrapper" style={{ color: 'red' }}> | ||
| <div key="a">Hello</div> | ||
| <div key="b">World</div> | ||
| </FlipMove> | ||
|
|
||
| // HTML | ||
| <div class="flip-wrapper" style="color: red;"> | ||
| <div key="a">Hello</div> | ||
| <div key="b">World</div> | ||
| </div> | ||
| ``` | ||
|
|
||
| You can supply a different element type with the `typeName` prop: | ||
|
|
||
| ```jsx | ||
| // JSX | ||
| <FlipMove typeName="ul"> | ||
| <li key="a">Hello</li> | ||
| <li key="b">World</li> | ||
| </FlipMove> | ||
|
|
||
| // HTML | ||
| <ul style="color: red;"> | ||
| <li key="a">Hello</li> | ||
| <li key="b">World</li> | ||
| </ul> | ||
| ``` | ||
|
|
||
| Finally, if you're using React 16 or higher, and Flip Move 2.10 or higher, you can use the new "wrapperless" mode. This takes advantage of a React Fiber feature, which allows us to omit this wrapping element: | ||
|
|
||
| ```jsx | ||
| // JSX | ||
| <div className="your-own-element"> | ||
| <FlipMove typeName={null}> | ||
| <div key="a">Hello</div> | ||
| <div key="b">World</div> | ||
| </FlipMove> | ||
| </div> | ||
|
|
||
| // HTML | ||
| <div class="your-own-element"> | ||
| <div key="a">Hello</div> | ||
| <div key="b">World</div> | ||
| </div> | ||
| ``` | ||
|
|
||
| Wrapperless mode is nice, because it makes FlipMove more "invisible", and makes it easier to integrate with parent-child CSS properties like flexbox. However, there are some things to note: | ||
|
|
||
| - This is a new feature in FlipMove, and isn't as battle-tested as the traditional method. Please test thoroughly before using in production, and report any bugs! | ||
| - FlipMove does some positioning magic for enter/exit animations - specifically, it temporarily applies `position: absolute` to its children. For this to work correctly, you'll need to make sure that `<FlipMove>` is within a container that has a non-static position (eg. `position: relative`), and no padding: | ||
|
There was a problem hiding this comment. This is something I noticed in Storybook; It's kind of a bummer :( and makes me think that wrapperless mode will likely cause more trouble than it's worth... but I figure we'll see. If we get a ton of confusing issues, maybe we can add a check that console.warns when the parent DOM element isn't configured correctly. There was a problem hiding this comment. since we already have the dom node, those styles could be overwritten after findDOMContainer = () => {
// eslint-disable-next-line react/no-find-dom-node
const domNode = ReactDOM.findDOMNode(this);
const parentNode = domNode && domNode.parentNode;
// This ought to be impossible, but handling it for Flow's sake.
if (!parentNode || !(parentNode instanceof HTMLElement)) {
return;
}
// Make sure parentNode has position relative and no padding
parentNode.style.position = 'relative';
parentNode.style.padding = 0;
this.parentData.domNode = parentNode;
};should be mentioned in the docs though. There was a problem hiding this comment. We can do that only if There was a problem hiding this comment. Yeah, so I thought about applying a "default" relative position if it isn't set to something other than So either we apply position: relative, which fixes FlipMove but breaks their layout, or we do nothing, and it breaks enter/exit animations. The latter is clearly a FlipMove issue, and so they'll come here and we can advise them... but the former might appear to them as a CSS issue / they won't understand why things are broken, why a random Maybe we can apply relative positioning if it's static, and also That said, it's really rare that Gonna do it in a subsequent PR to make reviews easier, but I'll apply @Hypnosphi's static check, and issue a console.warn when the change is necessary. |
||
|
|
||
| ```jsx | ||
| // BAD - this will cause children to jump to a new position before exiting: | ||
| <div style={{ padding: 20 }}> | ||
| <FlipMove typeName={null}> | ||
| <div key="a">Hello world</div> | ||
| </FlipMove> | ||
| </div> | ||
|
|
||
| // GOOD - a non-static position and a tight-fitting wrapper means children will | ||
| // stay in place while exiting: | ||
| <div style={{ position: 'relative' }}> | ||
| <FlipMove typeName={null}> | ||
| <div key="a">Hello world</div> | ||
| </FlipMove> | ||
| </div> | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -277,15 +277,17 @@ In general, it is advisable to ignore the `domNodes` argument and work with the | |
|
|
||
| ### `typeName` | ||
|
|
||
| | **Accepted Types:** | **Default Value** | | ||
| |----------------------|-------------------| | ||
| | `String` | `null` | 'div' | | ||
|
There was a problem hiding this comment. I mentioned this in #202, and didn't see any strong opinions; my preference is that we use If y'all think there's value in also supporting |
||
|
|
||
|
|
||
| Flip Move wraps your children in a container element. By default, this element is a `div`, but you may wish to provide a custom HTML element (for example, if your children are list items, you may wish to set this to `ul`). | ||
|
|
||
| Any valid HTML element type is accepted, but peculiar things may happen if you use an unconventional element. | ||
|
|
||
| With React 16, Flip Move can opt not to use a container element: set `typeName` to `null` to use this new "wrapperless" behaviour. [Read more](https://github.com/joshwcomeau/react-flip-move/blob/master/README.md#wrapping-elements). | ||
|
|
||
| --- | ||
|
|
||
| ### `disableAllAnimations` | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,23 @@ | ||
| declare module 'react-flip-move' { | ||
|
There was a problem hiding this comment. Argh, Prettier updated all the formatting here, adding trailing commas and single-quotes. @Hypnosphi does this matter? Also, I renamed this file from There was a problem hiding this comment. yeah, pretty sure there should be. we don't want to allow null as a value on versions lower than 2.10.x you can use wildcards in the name to span multiple versions though, like we're currently doing with the patch version. There was a problem hiding this comment.
If it doesn't for you, you shouldn't use prettier =) And I believe, trailing commas are one of the few things that are configurable There was a problem hiding this comment.
Yep, makes sense, thanks!
You misunderstand; I was asking specifically about But yeah, sounds like it's a non-issue, so I'll go ahead. There was a problem hiding this comment. Well we kinda introduce new API, so it requires an updated libdef There was a problem hiding this comment. And I think we should disable eslint for flow-typed directory so that it’s easier to sync it with the version in flow-typed repo |
||
| declare export type Styles = { | ||
| [key: string]: string, | ||
| }; | ||
|
|
||
| declare type ReactStyles = { | ||
| [key: string]: string | number, | ||
| }; | ||
|
|
||
| declare export type Animation = { | ||
| from: Styles, | ||
| to: Styles, | ||
| }; | ||
|
|
||
| declare export type Presets = { | ||
| elevator: Animation, | ||
| fade: Animation, | ||
| accordionVertical: Animation, | ||
| accordionHorizontal: Animation, | ||
| none: null, | ||
| }; | ||
|
|
||
| declare export type AnimationProp = $Keys<Presets> | boolean | Animation; | ||
|
|
@@ -28,27 +28,27 @@ declare module "react-flip-move" { | |
| bottom: number, | ||
| left: number, | ||
| height: number, | ||
| width: number, | ||
| }; | ||
|
|
||
| // can't use $Shape<React$Element<*>> here, because we use it in intersection | ||
| declare export type ElementShape = { | ||
| +type: $PropertyType<React$Element<*>, 'type'>, | ||
| +props: $PropertyType<React$Element<*>, 'props'>, | ||
| +key: $PropertyType<React$Element<*>, 'key'>, | ||
| +ref: $PropertyType<React$Element<*>, 'ref'>, | ||
| }; | ||
|
|
||
| declare type ChildHook = (element: ElementShape, node: ?HTMLElement) => mixed; | ||
|
|
||
| declare export type ChildrenHook = ( | ||
| elements: Array<ElementShape>, | ||
| nodes: Array<?HTMLElement>, | ||
| ) => mixed; | ||
|
|
||
| declare export type GetPosition = (node: HTMLElement) => ClientRect; | ||
|
|
||
| declare export type VerticalAlignment = 'top' | 'bottom'; | ||
|
|
||
| declare export type Child = void | null | boolean | React$Element<*>; | ||
|
|
||
|
|
@@ -61,7 +61,7 @@ declare module "react-flip-move" { | |
| disableAllAnimations: boolean, | ||
| getPosition: GetPosition, | ||
| maintainContainerHeight: boolean, | ||
| verticalAlignment: VerticalAlignment, | ||
| }; | ||
|
|
||
| declare type PolymorphicProps = { | ||
|
|
@@ -70,37 +70,36 @@ declare module "react-flip-move" { | |
| staggerDurationBy: string | number, | ||
| staggerDelayBy: string | number, | ||
| enterAnimation: AnimationProp, | ||
| leaveAnimation: AnimationProp, | ||
| }; | ||
|
|
||
| declare type Hooks = { | ||
| onStart?: ChildHook, | ||
| onFinish?: ChildHook, | ||
| onStartAll?: ChildrenHook, | ||
| onFinishAll?: ChildrenHook, | ||
| }; | ||
|
|
||
| declare export type DelegatedProps = { | ||
| style?: ReactStyles, | ||
| }; | ||
|
|
||
| declare export type FlipMoveDefaultProps = BaseProps & PolymorphicProps; | ||
|
|
||
| declare export type CommonProps = BaseProps & | ||
| Hooks & { | ||
| children?: ChildrenArray<Child>, | ||
| }; | ||
|
|
||
| declare export type FlipMoveProps = FlipMoveDefaultProps & | ||
| CommonProps & | ||
| DelegatedProps & { | ||
| appearAnimation?: AnimationProp, | ||
| disableAnimations?: boolean, // deprecated, use disableAllAnimations instead | ||
| }; | ||
|
|
||
| declare class FlipMove extends React$Component<FlipMoveProps> { | ||
| static defaultProps: FlipMoveDefaultProps, | ||
| } | ||
|
|
||
| declare export default typeof FlipMove | ||
|
|
||
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.
This TOC was quite outdated, and I realized it'll always be problematic. This doc isn't so long now that
API Referenceis its own thing, so I ditched the TOC.