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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frontity Flow component - Html2React experiment 馃И #67

Closed
wants to merge 3 commits into from

Conversation

michalczaplinski
Copy link
Member

@michalczaplinski michalczaplinski commented Mar 30, 2020

This branch shows an experiment that I attempted in order to allow to compose and style the gutenberg blocks with a more developer-friendly API.

https://www.loom.com/share/6c650ffbae2a4a8c9025a900d311db86

This experiment was prompted by the requirements for the Frontity Flow component of the website: #51

@frontibotito
Copy link
Member

frontibotito commented Mar 30, 2020

Deploy preview for website ready!

Built with commit 477a06a

https://frontity-org-43l2vwp9y.now.sh

@michalczaplinski
Copy link
Member Author

michalczaplinski commented Mar 30, 2020

I've added a second example, which shows in more detail the problem that I'm trying to solve.

@michalczaplinski michalczaplinski linked an issue Mar 31, 2020 that may be closed by this pull request
@michalczaplinski michalczaplinski changed the title Html2React experiment 馃И Frontity Flow component - Html2React experiment 馃И Mar 31, 2020
@luisherranz
Copy link
Member

luisherranz commented Apr 1, 2020

@michalczaplinski, I want to make sure I understood the experiment.

We have this layout:

<div class="switch">
  <div class="switch-button"></div>
  <div class="switch-value"></div>
</div>

and when the user clicks the "switch-button" div, the "switch-value" div appears of disappears, is that right?

@michalczaplinski
Copy link
Member Author

michalczaplinski commented Apr 1, 2020

@luisherranz that's right. Or maybe we want to render some text inside the div with switch-value class depending on the state of the switch.

Like:
now the button is on / off .

@luisherranz
Copy link
Member

luisherranz commented Apr 1, 2020

Ok, thanks :)

This is my take:

  • If the switch state needs to be internal to React, you need to share it from a parent. That can be solved with React context.

    • switch:
      • Create visible and setVisible with React.useState.
      • Create context, add those to the context and use the <Provider>.
    • switch-button:
      • Get setVisible from the context using useContext.
    • switch-value:
      • Get visible from the context.
  • If the switch state needs to be exposed in the state manager then there's no need for a context, just connect both components to the state manager, create an action for the switch and read the state from the "switch-value".

    • switch: nothing.
    • switch-button: use actions.theme.toggleSwitch().
    • switch-value: read from state.theme.isSwitchOn.

Does it makes sense?

@DAreRodz
Copy link
Member

DAreRodz commented Apr 1, 2020

I used the Frontity state to handle the header visibility in #63 (see index.ts).

The flow component is not goint to appear in every page like the header but maybe is a good idea to use our state manager anyway, it is pretty good actually. 馃檲

@DAreRodz
Copy link
Member

DAreRodz commented Apr 1, 2020

And for the switch-button and switch-value elements, I would propose to do that kind of stuff in the processor, maybe adding the match function you thought about to the nodes of the html tree.

Something like:

export const switchElement: Processor = {
  name: "switch",
  test: ({ node }) =>
    node.type === "element" &&
    node.props?.className?.split(" ").includes("switch"),
  processor: ({ node }) => {

    // `node.match` would return the elements that match the query
    const switchButtonNodes = node.match("div.switchButton");
    const switchContentNodes = node.match("div.content");

    // Replace `div` by our components in that nodes 
    const switchButtons = switchButtonNodes.map(node => ({
      ...node,
      component: SwitchButton
    }));
    const switchContent = switchContentNodes.map(node => ({
      ...node,
      component: SwitchContent
    }))

    return {
      component: Switch,
      children: [
        // Add the nodes to `children` so they will be rendered by Html2React
        ...switchButtons,
        ...switchContent,
      ]
    };
  },
};

EDIT: this way you won't need to nest Html2React components 馃憤

@michalczaplinski
Copy link
Member Author

michalczaplinski commented Apr 2, 2020

Thanks guys! 馃檪

I see what you both mean. I know that I could use React context to pass state around the component tree. Actually I didn't think about using connect() but that's because I think this state is not global and does not really belong there even though I know this would "solve" the problem :)

To me it seems like we're intentionally missing out on one of the best features of react here (the ability to pass different props to child components depending on some condition). And instead we're accomplishing the same but in a worse way 馃槄.

If you squint your eyes this whole part looks almost like a react component in it's own right:

  processor: ({ node }) => {

    // `node.match` would return the elements that match the query
    const switchButtonNodes = node.match("div.switchButton");
    const switchContentNodes = node.match("div.content");

    // Replace `div` by our components in that nodes 
    const switchButtons = switchButtonNodes.map(node => ({
      ...node,
      component: SwitchButton
    }));
    const switchContent = switchContentNodes.map(node => ({
      ...node,
      component: SwitchContent
    }))

    return {
      component: Switch,
      children: [
        // Add the nodes to `children` so they will be rendered by Html2React
        ...switchButtons,
        ...switchContent,
      ]
    };
  },

We should be able to just use JSX and props to accomplish the same task in a way that is more familiar to react developers!

I ll move the discussion to a Feature Discussion because I still think we can do better 馃檪

@michalczaplinski
Copy link
Member Author

michalczaplinski commented Apr 2, 2020

We are moving this conversation to: https://community.frontity.org/t/gutenberg-frontity-integration/1467

@luisherranz luisherranz closed this Apr 2, 2020
@luisherranz luisherranz removed their request for review April 2, 2020 08:24
@luisherranz luisherranz removed a link to an issue Apr 7, 2020
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