Skip to content

Conversation

Ldoppea
Copy link
Member

@Ldoppea Ldoppea commented Jun 22, 2023

This PR is a proposal for a refactoring on our WebView management.

What I'm waiting from you are some feedback on the proposal, if you think is a good idea or not. I'm currently stuck on the implementation and I don't want to invest more time until I'm sure it is the good direction.

Problem

Today our codebase structure is the following:

HomeView
└ CozyProxyWebView
   └ CozyWebView
      └ ReloadInterceptorWebView
         └ SupervisedWebView

Each WebView component has a hard-coded reference to its WebView child (i.e CozyWebView defines that its child is a ReloadInterceptorWebView)

This makes difficult to add a layer to the WebView's tree, or to invert some of them in the hierarchy.

Imagine that we want to add a ReloadableWebView that embed ALL of our webviews and allows to unmount/remount everything when we want a reload (so we enforce the proxywebview to generate a new HTML)

To make this we would have to edit HomeView to use a ReloadableWebView instead of CozyProxyWebView, to hard code ReloadableWebView with CozyProxyWebView as child, and then we should remember to search for all other CozyProxyWebView references in the code and replace CozyProxyWebView by ReloadableWebView like we did in HomeView

Here would be the new codebase structure

HomeView
└ ReloadableWebView
  └ CozyProxyWebView
     └ CozyWebView
        └ ReloadInterceptorWebView
           └ SupervisedWebView

So my question is How can we make this more flexible?

Proposal

My proposal is to create a ComposedWebView that would allow developers to "compose" their WebView like they want.

The API would be like this:

return (
  <ComposedWebView
      {...props}
      childrenTypes={[
        CozyProxyWebView,
        CozyWebView,
        ReloadInterceptorWebView,
        SupervisedWebView
      ]}
    />
)

Which would be translated on runtime as the following

CozyProxyWebView
└ CozyWebView
   └ ReloadInterceptorWebView
      └ SupervisedWebView

This would allow to easily add/remove parts. If we want to remove the ReloadInterceptorWebView from the generated tree then we can do:

return (
  <ComposedWebView
      {...props}
      childrenTypes={[
        CozyProxyWebView,
        CozyWebView,
        //ReloadInterceptorWebView,
        SupervisedWebView
      ]}
    />
)

If we want to add a ReloadableWebView on the top level then we can do:

return (
  <ComposedWebView
      {...props}
      childrenTypes={[
        ReloadableWebView,
        CozyProxyWebView,
        CozyWebView,
        ReloadInterceptorWebView,
        SupervisedWebView
      ]}
    />
)

Work in progress

This PR is a draft implementation for this concept.

src/components/webviews/ComposedWebView.js contains the logic that translates the flat declaration into a nested WebView tree on runtime.

This would have some repercussions on the way we create WebViews:

  • WebView components must be wrapped in a forwardRef
  • WebView components must take a ChildWebview prop defaulted to WebView and use it as their child
  • All WebView methods like onShouldStartLoadWithRequest should consider they can have a parent and call it if exists
    • Example:
<ChildWebView
  onShouldStartLoadWithRequest={(initialRequest) => {
    if (/* some condition */) {
      // do stuff
      return false
    }

    if (props.onShouldStartLoadWithRequest) {
      return props.onShouldStartLoadWithRequest(initialRequest)
    }  else {
      return true
    }
  }}
/>

⚠️ Current implementation is not complete and has some issues:

  • ReloadInterceptorWebView gets nodeHandle errors
    • but if we remove it from the tree then everything seems to work
  • CozyAppScreen will do an infinite loop
    • however HomeView seems to work correctly

@Crash--
Copy link
Contributor

Crash-- commented Jun 22, 2023

Maybe linked to #411 ?

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.

2 participants