Feature/with react #355

AndyOGo · 2018-03-21T10:41:53Z

Fixes #330 .

Changes proposed in this pull request:

adds withReact thunk
quickfixes the demo build (adds scss, jsx rosolve, ...)
adds todomvc inspired by http://todomvc.com/
adds trigger of custom events for limited components (needs to be done for all)

Type of change

Please delete options that are not relevant.

New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Locally

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

LucaMele

Please update the Readme.md with a short code sample on how to use it (not in the wiki).

Same style and under the one already existing on how to include WC nativly

LucaMele · 2018-03-28T12:39:32Z

src/js/with-react.jsx

+      const { wcNode } = this;
+
+      Object.keys(props).forEach((key) => {
+        if (key === 'children' || key === 'style') {


better an external enum with an array of properties

and also maybe use this on the property setting on the WC abstract component

Do I understand you correctly, that I should put 'children' and 'style' into a blacklist array? If so absolutely

AndyOGo · 2018-03-28T16:32:31Z

@LucaMele
Regarding docs I have already added an @example doclet.
But sure I can also add it to the readme/contribution file

AndyOGo · 2018-03-29T08:13:15Z

hmh, adding a nested <input> within WebComponents seem to lose focus on edit 🤔

AndyOGo · 2018-03-29T09:31:11Z

src/demos/todomvc/todo-header.jsx

+const AXAHeaderMainReact = withReactBound(AXAHeaderMain);
+const AXAHeaderLogoReact = withReactBound(AXAHeaderLogo);
+
+// @todo: super hacky way to keep the input's focus within custom elements


@LucaMele
@TheDadi
I distilled a major problem with reactified custom elements consuming <input> children from React. Basically on every re-render the focus is lost of the input control, allowing the user to type only one character at a time 😱

The re-rendering of the parent web-component, removes the whole DOM sub tree at first and the re-inserts it again. This causes the loss of the whole browser intrinsic <input>'s state, like:

focus

cursor position

range selection,

etc.

const Example = (({ onChange, value, }) => ( <AXAHeader> <AXAHeaderMain> // the following input control will lose focus at every onKeyup/onClick causing a re-render <input type="text" onChange={onChange} value={value} /> </AXAHeaderMain> </AXAHeader> );

mmmm.. not good.. what can we do? and why do we re-render? Can we do sort of one-way-binding?

We re-render because on every input's onChange react's state changes - so React triggers a re-render (which is normal behavior for controlled inputs).
The web-components unfortunately detach completely from the DOM (if you remember the deletion speed test) and attaches a newly generated DOM tree...
So I guess diffing should help us here, but need to verify that...

@LucaMele
I found the main glitch. Currently any web-component like <axa-foo bar="baz"> would have even re-rendered if the attribute was set to the exact same value, like:

axaFoo.setAttribute('bar', 'baz'); // no change -> should not trigger re-render, but did before axaFoo.bar = 'baz'; // no change -> should not trigger re-render, but did before

I fixed this now by checking for shallow equality. And this fixed the input issue too:)

AndyOGo · 2018-03-29T18:01:44Z

Okay that's a super success story, except the <input> state issue...

AndyOGo · 2018-03-29T18:30:14Z

src/components/m-button/js/button.js

+    const cancelled = fire(this.wcNode, 'axa-click', null, { bubbles: true, cancelable: true, composed: true });
+
+    if (!cancelled) {
+      event.preventDefault();


@LucaMele
preventDefault the original event works :)

AndyOGo · 2018-03-29T18:30:53Z

src/components/m-footer-links/js/footer-links.js

+
+  handleClick(event, delegateTarget) {
+    // @todo: would be cool to be able to use props here, cause now it needs JSON.parse...
+    const index = getAttribute(delegateTarget, 'index');


@LucaMele
Passing the current item's data down to the custom event is possible 👍

AndyOGo · 2018-03-29T18:31:45Z

@LucaMele
Now all components need to be examined for custom events and their associated data.

AndyOGo · 2018-03-29T18:32:56Z

src/js/with-react.jsx

+
+          const eventName = dasherize(`${keyFrom2}${key.substring(3)}`);
+
+          eventCache[key] = on(wcNode, eventName, props[key], { passive });


@LucaMele
I think it could be convenient to extract the event.detail here optionally, so that consumers can just deal with the data they need.

AndyOGo · 2018-03-30T06:10:52Z

src/js/abstract/base-component.js

    const key = camelize(name);

    this[key] = toProp(newValue);
  }

+  /**


@LucaMele
This is suuuper important! It makes sure that the component only re-renders if any attribute has actually changed:)
And it mitigates the nested <input> issue afore mentioned 🎉

AndyOGo · 2018-03-30T06:19:24Z

@LucaMele
This is ready to be merged :)

AndyOGo · 2018-05-01T09:20:39Z

I just rebased and it works in chrome with incremental rendering of web-comonents :)

AndyOGo · 2018-05-01T09:42:37Z

I created this follow-up issue #418

AndyOGo · 2018-05-01T09:47:24Z

are there any objections for this to merged?

AndyOGo added Enhancement High-prio labels Mar 21, 2018

AndyOGo self-assigned this Mar 21, 2018

AndyOGo requested review from LucaMele and TheDadi March 21, 2018 10:41

AndyOGo force-pushed the feature/with-react branch 8 times, most recently from 0d16d85 to 1b67be0 Compare March 28, 2018 10:35

AndyOGo mentioned this pull request Mar 28, 2018

Feature/form components #236

Closed

LucaMele suggested changes Mar 28, 2018

View reviewed changes

AndyOGo force-pushed the feature/with-react branch 2 times, most recently from 9529cdf to e517479 Compare March 28, 2018 14:38

AndyOGo force-pushed the feature/with-react branch from b8af9d5 to 73f0fa2 Compare March 29, 2018 08:23

AndyOGo commented Mar 29, 2018

View reviewed changes

AndyOGo force-pushed the feature/with-react branch from e821346 to 707d178 Compare March 29, 2018 11:09

AndyOGo commented Mar 29, 2018

View reviewed changes

AndyOGo commented Mar 30, 2018

View reviewed changes

AndyOGo and others added 25 commits May 1, 2018 10:59

removed the input hack

03b5231

renamed to shouldUpdateCallback

405b19f

adding array partition reducer

2397c93

start adding batch props for integrations

c770d6a

improved FOUC by batch processing of props

a9422a3

added docs for batch props

804617a

fixed initial attribute fetching

373c1f1

added updateProp for batching

da1db52

add some props changes

ef3a5d9

added doclet

55a307d

added active item to footer links

058151a

switched ghost not ghost for todo item completion

f9abf30

renamed this private func

6fe9a8e

added todo help to footer

a1b1222

improved docs of events

6486549

utilised peer deps for with react

1cc7a08

just a typo

25f1f44

React 14 should also work

2b52339

fixed missing context

df6c7a1

start adding event doclets

92bf445

moar doclets

7dead93

add mobile menu react entry

45f6370

use name prop

e726f0c

added function name utility

02436e0

fixed IE broken func name prop

284bc69

AndyOGo merged commit abc9a66 into develop May 1, 2018

AndyOGo deleted the feature/with-react branch May 1, 2018 10:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/with react #355

Feature/with react #355

AndyOGo commented Mar 21, 2018 •

edited

Loading

LucaMele left a comment

LucaMele Mar 28, 2018

LucaMele Mar 28, 2018

AndyOGo Mar 28, 2018

AndyOGo Mar 28, 2018

AndyOGo commented Mar 28, 2018

AndyOGo commented Mar 29, 2018 •

edited

Loading

AndyOGo Mar 29, 2018 •

edited

Loading

LucaMele Mar 29, 2018

AndyOGo Mar 29, 2018 •

edited

Loading

AndyOGo Mar 30, 2018

AndyOGo commented Mar 29, 2018

AndyOGo Mar 29, 2018

AndyOGo Mar 29, 2018

AndyOGo commented Mar 29, 2018

AndyOGo Mar 29, 2018

AndyOGo Mar 30, 2018 •

edited

Loading

AndyOGo commented Mar 30, 2018

AndyOGo commented May 1, 2018

AndyOGo commented May 1, 2018

AndyOGo commented May 1, 2018


		const eventName = dasherize(`${keyFrom2}${key.substring(3)}`);

		eventCache[key] = on(wcNode, eventName, props[key], { passive });

Feature/with react #355

Feature/with react #355

Conversation

AndyOGo commented Mar 21, 2018 • edited Loading

Type of change

How Has This Been Tested?

Checklist:

LucaMele left a 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

AndyOGo commented Mar 28, 2018

AndyOGo commented Mar 29, 2018 • edited Loading

AndyOGo Mar 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyOGo Mar 29, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyOGo commented Mar 29, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndyOGo commented Mar 29, 2018

Choose a reason for hiding this comment

AndyOGo Mar 30, 2018 • edited Loading

Choose a reason for hiding this comment

AndyOGo commented Mar 30, 2018

AndyOGo commented May 1, 2018

AndyOGo commented May 1, 2018

AndyOGo commented May 1, 2018

AndyOGo commented Mar 21, 2018 •

edited

Loading

AndyOGo commented Mar 29, 2018 •

edited

Loading

AndyOGo Mar 29, 2018 •

edited

Loading

AndyOGo Mar 29, 2018 •

edited

Loading

AndyOGo Mar 30, 2018 •

edited

Loading