Fix frameworks using constant context value

[artmanque]

react/angular/angular2/vue/vue3 components call render method using a constant CONTEXT.IFRAME overriding user's defaultContext from create options
Fixes #393

[bluepnume]

This is by design -- React/angular/etc. components are inline by definition, so need to be in iframe mode. To open a popup you would need to wait for a user event like a click, then manually call MyComponent({ ... }).render()

[artmanque]

@bluepnume the render method by default uses an iframe. Forcibly passing the context constant not only overrides the defaultContext option on the component's definition but also makes the docs misleading and this option obselete altogether
https://github.com/krakenjs/zoid/blob/main/docs/api/create.md#defaultcontext-string

[bluepnume]

Sure but defaultContext means "it will pick this context by default" not "it will pick this context always". If you render a React component, it's like you're implicitly saying "this component should render inline in this case, so I want to override the default".

Example:

• You have a component
• If a user clicks on a button, you want to open it in a popup -> so you call .render() manually
• If there is no click, you want to fall back to an iframe -> so you render using the React driver, knowing that is implicitly inline

[artmanque]

It is quite clear by the name defaultContext that if you don't specify context to render method, then it should use the defaultContext in my component's definition unlike the current scenario.

This is quite problematic and unexpected because, when a driver renders the component, it overrides the component's defaultContext option. The app has to do another explicit render with a context causing glitches.

Example:

1. Component definition: defaultContext: popup
2. User triggers an event
3. Driver mounts the component by default in iframe, disregarding defaultContext option in component def.
4. Event listener grabs the reference to rendered component's parent and triggers render method on it again now with context=popup

[bluepnume]

I think I'm not quite following -- why would the event listener use the driver in step 3, but then manually call render in step 4 -- wouldn't it pick one or the other consistently? Can you share a small code sample of what you mean here?

[artmanque]

you understand my point correctly.

When using the driver pattern, the app cannot control the context in render method as it is using a constant value (iframe) instead of defaultContext option from component def, which would be handled automatically by the render method if the context argument was not passed explicitly.

This PR addresses the explicit argument context=IFRAME being passed to render method in the driver pattern, so that defaultContext option can be leveraged as it should.

[bluepnume]

I guess let me rephrase the question; what do you want to happen with a driver render in the case that defaultContext is a popup? Do you want zoid to render a popup in that case?

[artmanque]

Yes.

Currently it enforces an iframe:

zoid/src/drivers/vue.js

Line 62 in eb8677a

this.parent.render(el, CONTEXT.IFRAME);

[bluepnume]

Right, so that's the underlying problem here. Normally browsers only allow you to open a popup on a user-initiated click or keyboard event -- and only synchronously. Whereas many rendering libraries, including I think React, do not synchronously render on click. That will mean a lot of the time your popup will completely blocked from opening by the browser.

So if you want to open a popup, my recommendation would be to explicitly listen for a click or keyboard event, then manually call MyComponent({ ... }).render() rather than using a driver.

[artmanque]

That's a fair work around. Regardless of how it should be done, the defaultContext value should be coming from component's definition instead being hard coded as it currently.

The default value of defaultContext is iframe. This PR addresses it.

[bluepnume]

I'm really not sure I agree. The logic as it stands is:

1. If the caller signals their intent to render to a specific context, use that context. e.g.
   a) The caller explicitly passed in a context when rendering, or
   b) The caller rendered the component using a driver that is implicitly inline (and does not even work in popup mode)
2. If there is no explicit or implicit intent at render-time, fall back to the defaultContext
3. If there is no defaultContext, fall back to iframe

Not to mention -- this is a backwards incompatible change, right? Given that there could be zoid components out there which are currently relying on that 1.b. behavior of "Render to a popup by default, but fall back to an inline iframe if the caller uses a driver (since in that case the in-built browser popup-blocker will be triggered)".

What is the actual use-case you're hoping to unblock here? Are you trying to render a popup through one of the drivers, and are you not getting popup-blocked when you test this?

[artmanque]

My use case is I cannot render a popup using the driver pattern as it forces an iframe over component definition options.

The code that I am targeting is only driver.register for all the supported frameworks. Your description is accurate for the rest. In the driver pattern:

1. The component is rendered as soon as it gets mounted.
   a) The driver code explicitly passes iframe context argument to render (as referred above), overriding the defaultContext from component definition options. The default value of defaultContext is always iframe so explicitly passing iframe to render() only overrides the defaultContext option stated in component definition.
   b) The component gets in a popup mode just fine and renders the url from component definition.
2. The driver pattern does not fallback to defaultContext at render-time as the render method was called with context=iframe explicitly.
3. The default value of defaultContext is iframe so the driver.register should not pass context argument explicitly.

This is backwards compatible in that, it will render to a popup by default, but will fall back to an inline iframe if the caller uses a driver since the default value of defaultContext will be iframe always.

References:

zoid/src/component/component.js

Line 354 in eb8677a

const getDefaultContext = (props : PropsInputType<P>, context : ?$Values<typeof CONTEXT>) : ZalgoPromise<$Values<typeof CONTEXT>> => {

zoid/src/component/component.js

Line 184 in eb8677a

defaultContext = CONTEXT.IFRAME,

[bluepnume]

Why do you want to render a popup window using the React driver when it will be blocked by the browser?

This is what happens when I run a test locally to do this:

[artmanque]

That is an artifact. It's not always the case. My browser on the other hand allows popup by default.

I want the React driver to adopt defaultContext option from my component's definition which it overrides silently which makes it confusing and misleading

[bluepnume]

It's not an artifact, I see the exact same behavior in Chrome, Safari and Firefox:

Which specific browser and version are you testing in? And did you allow an exception in the browser to prevent popup blocking?

[bluepnume]

https://stackoverflow.com/questions/2587677/avoid-browser-popup-blockers

https://thewebdev.info/2021/04/24/how-to-avoid-browser-popup-blockers-with-javascript-code/

[artmanque]

It is an artifact because this behavior is a result of

1. User's browser preferences
2. How the app triggered render on driver component.

Also, notice that I have not allowed the popup by default. I am using chrome and my apps renders the driver component only when the user does an action.

It is problematic because the driver then does not use my defaultContext option. It is an over-generalization to enforce the driver's behavior to use an iframe while the user has different preferences.

Notice that it is not necessary for the driver to explicitly pass iframe to render, as the render method is going to resolve the finalContext from getDefaultContext which will return the context argument if valid or the defaultContext if any with the default value iframe

References:

zoid/src/drivers/angular2.js

Line 85 in eb8677a

this.parent.render(targetElement, CONTEXT.IFRAME);

zoid/src/component/component.js

Line 354 in eb8677a

const getDefaultContext = (props : PropsInputType<P>, context : ?$Values<typeof CONTEXT>) : ZalgoPromise<$Values<typeof CONTEXT>> => {

zoid/src/component/component.js

Line 184 in eb8677a

defaultContext = CONTEXT.IFRAME,

[bluepnume]

It is an artifact because this behavior is a result of

User's browser preferences

Sure, but the default behavior is to block in every browser I've ever used. Are you working in an environment where users of your component will have to change their browser settings before using your component?

How the app triggered render on driver component.

Not really - libraries like React force components to be asynchronously rendered, right? So even using a click event won't help much here.

[artmanque]

This sounds like an over generalisation to optimize user experience essentially by handicapping the app to render a popup when using the driver (More at end).

The current design violates two SOLID principles (Learn More):

1. Single-responsibility principle.

The documentation clearly states that defaultContext is responsible for setting the context during render. The same is not reflected when the driver renders the component thus the driver takes off responsibility to itself handicapping the app of the ability to do so.

2. Dependency inversion principle

The driver code assumes of a concrete scenario that the user's browser has certain fixed behavior and changes the abstract behavior of zoid (i.e render should use CONTEXT.IFRAME) that is not the original intent of the app.

FYI; all versions of react do not force asynchronous rendering. Learn More
A Live Demo how with-react-driver renders a popup without the browser blocking and context=iframe not hardcoded inside driver.register

[bluepnume]

Thanks for the demo! OK I'm convinced. If you can trigger a synchronous render through React, that's good enough for me.

That said: there is still a backwards-compatibility question here; there could be users of zoid who are currently relying on the behavior of defaulting to a popup, but falling back to an iframe when using any one of the drivers.

So let's brainstorm a little on an interface that can allow us to override the default behavior and also preserve backwards compatibility. A few ideas:

• A new defaultDriverContext option
• Allow passing context as a prop
• Allow passing context when calling .driver()

[artmanque]

Currently, the defaultContext option is being overridden by the driver.

That said: there is still a backwards-compatibility question here; there could be users of zoid who are currently relying on the behavior of defaulting to a popup, but falling back to an iframe when using any one of the drivers.

*Correction: notice that currently the behavior is default to iframe (not popup) which is in alignment with what the driver is doing.
The default value of defaultContext is already iframe, so removing the explicit argument would really correct the currently anomalous behavior of zoid's consumer app only if:

1. App is using driver
2. defaultContext=popup is set in the component definition options

Which is the actual intent of the app i.e render popup using driver, as opposed to being overridden by the driver with an iframe.

Parent props should have a context option so that the app can control the context during render cycle as well

[bluepnume]

Parent props should have context option so that the app can control the context during render cycle

I think I'd be happy with this solution. It's not really a traditional 'prop' in that it's not needed in the child window/frame. But it avoids any backwards compatibility problems.

[artmanque]

Please take a look at the changes now. Here is a brief summary of this PR:

1. Adds parent prop context
2. Drivers call render method with context value from component's props
3. Fix demos to add dimensions. Currently an invalid value undefined is used for height/width causing the component to have no height/width. Environment: MacOS Chrome 98
4. Fix vue + angular2_Typescript demos to use correct script's bundle url

[codecov-commenter]

Codecov Report

Merging #397 (f28473b) into main (eb8677a) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #397   +/-   ##
=======================================
  Coverage   95.51%   95.52%           
=======================================
  Files          18       18           
  Lines        1182     1184    +2     
=======================================
+ Hits         1129     1131    +2     
  Misses         53       53           

Impacted Files	Coverage Δ
src/component/props.js	93.93% <ø> (ø)
src/drivers/angular.js	95.83% <100.00%> (+0.18%)	⬆️
src/drivers/angular2.js	93.75% <100.00%> (+0.20%)	⬆️
src/drivers/react.js	100.00% <100.00%> (ø)
src/drivers/vue.js	100.00% <100.00%> (ø)
src/drivers/vue3.js	87.50% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb8677a...f28473b. Read the comment docs.

[artmanque]

@bluepnume please update on this

[JoseIsra]

Hello sir @artmanque, i'm using zoid with vue3, do you know how i can configure the component correctly? the error that i have say

My configuration is this
`
import * as zoid from 'zoid/dist/zoid.frameworks';
const DemoBreakout = zoid.create({
tag: 'a-simple-tag',
url: 'some-url',
dimensions: {
width: '100%',
height: '100%',
},
attributes: {
iframe: {
scrolling: 'no',
allow: 'camera *; microphone *; display-capture *;',
allowFullScreen: 'true',
allowusermedia: 'true',
},
},
props: {
gamesList: {
type: 'array',
required: false,
},
gameQuestions: { type: 'array', required: false },
dispatchQuestionForGame: {
type: 'function',
required: false,
},
startWithVideo: {
type: 'boolean',
required: false,
},
},
});
const Breakout = DemoBreakout.driver('vue3');

export default Breakout;
`
Your PR is the solution for this or this is something different?. Zoid documentation is really bad and the vue demo is broken

[artmanque]

@JoseIsra i suggest you to try running vue3 example from this PR before testing it in your app

[JoseIsra]

@artmanque how can i use zoid from its own repository. Do you have some sample code to follow? I can clone the repo and run the setup script, the package json says that. But after that? I'm new in this library-repository use case u.u