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

fix: specify dimensions in iframe demo #375

Merged
merged 1 commit into from
Oct 20, 2021
Merged

fix: specify dimensions in iframe demo #375

merged 1 commit into from
Oct 20, 2021

Conversation

crookedneighbor
Copy link
Contributor

If you go to https://krakenjs.com/zoid/demo/basic/iframe/index.htm, you'll see that the iframe login is missing.

This is because the dimensions aren't being passed, so the height and width are being set to undefined.

Passing the dimensions fixes the issue:

Screen Shot 2021-10-20 at 3 59 27 PM

A broader question I have is whether or not dimensions should be a required attribute or not. Currently, it's not marked as required in the docs: https://github.com/krakenjs/zoid/blob/master/docs/api/create.md#dimensions--width--string-height--string-

@crookedneighbor crookedneighbor requested a review from a team as a code owner October 20, 2021 21:00
@westeezy westeezy merged commit a4c1564 into krakenjs:master Oct 20, 2021
@bluepnume
Copy link
Collaborator

Thanks! Yeah dimensions should not be required, they should be defaulted when not passed. Can you take a look why the defaults are not kicking in?

@crookedneighbor
Copy link
Contributor Author

@bluepnume looks to me like default dimensions are being applied for popups, but not iframes:

zoid/src/parent/parent.js

Lines 621 to 622 in a4c1564

} else if (context === CONTEXT.POPUP && __ZOID__.__POPUP_SUPPORT__) {
let { width = DEFAULT_DIMENSIONS.WIDTH, height = DEFAULT_DIMENSIONS.HEIGHT } = getDimensions();

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