Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create contact intent #155

Merged
merged 14 commits into from
May 24, 2018
Merged

Create contact intent #155

merged 14 commits into from
May 24, 2018

Conversation

y-lohse
Copy link
Contributor

@y-lohse y-lohse commented May 23, 2018

Adds a second intent that uses the ContactForm component to allow creating a contact through an intent.
If the intent data has a flag me: true, we add that information to the contact's metadata. This contact becomes the assumed profile for the cozy's owner and get's a little "me" 🎏 in the list.

Since we now have 2 intents in this app, i've restructured things slightly, using a technique borrowed from cozy-store.

@y-lohse y-lohse self-assigned this May 23, 2018
@@ -6,6 +6,7 @@ import ContactFormField from "./ContactFormField";
import ContactFieldInput from "./ContactFieldInput";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little extra with this PR: we want to render the buttons for the creation form… elsewhere. Generally in a fixed modal footer.

I've used portals to make that possible, and successfully used it in the intent. However, when using this with the ContactFormModal, we run into a preact-bug when closing the Modal, and I'm too lazy to report it.

@@ -0,0 +1,93 @@
import React, { Children, Component } from "react";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This whole file comes from cozy-store, and while the internals are what they are, the API is pretty sweet.

@y-lohse
Copy link
Contributor Author

y-lohse commented May 23, 2018

Copy link
Contributor

@enguerran enguerran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work, maybe some changes can be made.

label={"Create a Contact (intent)"}
action="CREATE"
data={{ me: true }}
/>
Copy link
Contributor

@enguerran enguerran May 24, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To easily remove these lines later on, we should create a <FakeIntentToolbar /> whose display is controlled by fakeintent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeeeaaah I actually tried that, but then you have to wrap the buttons in a div, and the whole layout breaks :|

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tips: you can return a Array of components that will be added in the render tree as sibling.

class ArrayOfHello extends React.Component {
  render() {
    return [...Array(100)].map((item, index) => (
      <Hello name={`Mister ${index}`} />
    ));
  }
}

const App = () => (
  <div style={styles}>
    <Hello name="CodeSandbox" />
    <h2>Start editing to see some magic happen {"\u2728"}</h2>
    {true && <ArrayOfHello />}
  </div>
);

https://codesandbox.io/s/q73kkr1k26

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that works in preact. IIRc it's a react 16 feature that hasn't been ported yet!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right I overzealously replaced the function with a functional component.
Here is the code with a function in preact:
https://codesandbox.io/s/v60w5ppm45

@@ -77,6 +78,21 @@ const initialFieldValues = fields.reduce((initialValues, { name, isArray }) => {
return initialValues;
}, {});

const ContactFormFooterWrapper = ({ portalInto, children }) => {
return portalInto ? (
<Portal into={portalInto}>{children}</Portal>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you use a portal, but, hey, if it works, that sounds good.

};

const MeMarker = (props, { t }) => (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really fan of this name, but it is hard to find a good name, maybe a myselfTag or myselfLabel.

@@ -88,7 +100,7 @@ const ContactRow = props => {
onSelect={props.selection.onSelect}
/>
)}
<ContactIdentity name={name} />
<ContactIdentity name={name} me={me} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe myself is better than me to speak about user identity.

@enguerran
Copy link
Contributor

Ahah, MacOS filesystem is not case sensitive (d449b49) ;)

@@ -0,0 +1,93 @@
import React, { Children, Component } from "react";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like my previous comments disappeared — this file is largely borrowed from cozy-store. It allows for a nice intent handling in the app.

Copy link
Contributor

@goldoraf goldoraf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@y-lohse y-lohse merged commit 053c743 into cozy:master May 24, 2018
@y-lohse y-lohse deleted the create-contact branch May 24, 2018 17:19
CPatchane pushed a commit that referenced this pull request May 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants