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

added useConnect clarification injectProps #71

Merged
merged 7 commits into from May 25, 2020

Conversation

juanmaguitar
Copy link
Member

No description provided.

Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

Great work. I added a copule of commits but I don't know how to add a warning in gitbook syntax.

@@ -124,11 +136,10 @@ const { state, actions, libraries } = useConnect();

It's a React hook that returns the Frontity state, allowing the component to consume `state`, `actions` and `libraries` in components without passing them as props.
Copy link
Member

Choose a reason for hiding this comment

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

Could you please add a warning here so people understand that they still need to use connect?

You also indicate that connect improves the performance (so people don't ask why they need to also add connect`):

  • It optimizes your components with memo, so they won't re-render whenever a parent component re-renders.
  • It makes them reactive, so they will re-render when the parts of state they use are changed.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done! 👍

**Using `<Global>` for other than HTML tags is not recommended** because Frontity is not able to optimize it.
That means you can use it for tags like `html`, `body` , `a`, `img`, and so on...
But **avoid it for classes**.
Use either the CSS prop or styled-components instead.
Copy link
Member

Choose a reason for hiding this comment

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

We used styled components when referring to components styled with styled instead of styled-components to avoid confusion with the styled-components library, because we don't use that, we use Emotion. So we felt like styled components was more generic, and styled-components more specific to the library.

Anyway, up to you guys now. I just wanted to tell you the reason 🙂

Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

Thanks @juanmaguitar.

@@ -137,6 +137,16 @@ const { state, actions, libraries } = useConnect();
It's a React hook that returns the Frontity state, allowing the component to consume `state`, `actions` and `libraries` in components without passing them as props.


{% hint style="warning" %}

You still need to use `connect` to using `useConnect` properly.
Copy link
Member

Choose a reason for hiding this comment

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

I think there's a typo:

You still need to use connect when using useConnect.

@DAreRodz DAreRodz removed their request for review May 22, 2020 14:31
@DAreRodz
Copy link
Member

I removed myself from reviewers (I guess two approved reviews are enough 😄).

@juanmaguitar juanmaguitar merged commit ba0b2b4 into code May 25, 2020
@juanmaguitar juanmaguitar deleted the useConnect-clarifying-code branch May 25, 2020 10:17
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.

None yet

4 participants