Skip to content

track the fact that a canvas is provided so we don't overwrite its size #1322

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Apr 19, 2016
Merged

track the fact that a canvas is provided so we don't overwrite its size #1322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1cb1725
track the fact that a canvas is provided so we don't overwrite it's size
enjalot Apr 1, 2016
90f343f
use data attributed to track whether canvas is provided
enjalot Apr 1, 2016
78fdc3e
switch canvas provided boolean to mean a-frame provided it
enjalot Apr 1, 2016
c44ac63
add comments around provided canvas tracking
enjalot Apr 1, 2016
154452a
first draft of an embedded scenes example, utilizing user-provided ca…
enjalot Apr 1, 2016
aab0734
code style cleanup
enjalot Apr 2, 2016
7a82e7d
clean up embedded scene example
enjalot Apr 15, 2016
d42ce03
change aframeProvided -> aframeDefault
enjalot Apr 15, 2016
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 54 additions & 0 deletions examples/boilerplate-embedded-scenes/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<title>Embedded scene • A-Frame</title>
<meta name="description" content="Embedded scene • A-Frame">
<script src="../../dist/aframe.js"></script>
<style>
#mycanvas {
width: 200px;
height: 200px;
}
</style>
</head>
<body>
<h2>Through the looking glass.</h2>
<!-- this canvas element may be anywhere in the document -->
<canvas id="mycanvas"></canvas>
Copy link
Member

@ngokevin ngokevin Apr 1, 2016

Choose a reason for hiding this comment

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

Maybe move the canvas into the scene element?
Copy link
Contributor

@cvan cvan Apr 2, 2016

Choose a reason for hiding this comment

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

I think it's fine to show that that's not necessary
Copy link
Member

@ngokevin ngokevin Apr 2, 2016

Choose a reason for hiding this comment

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

I'd encourage it to wrap everything up nicely.

<p>’Twas brillig, and the slithy toves
Did gyre and gimble in the wabe:
All mimsy were the borogoves,
And the mome raths outgrabe.
“Beware the Jabberwock, my son!
The jaws that bite, the claws that catch!
Beware the Jubjub bird, and shun
The frumious Bandersnatch!”
</p>

<!-- This scene will be rendered inside the referenced canvas element -->
<a-scene canvas="canvas: #mycanvas">
<a-sphere position="0 1.25 -1" radius="1.25" color="#EF2D5E"></a-sphere>
<a-box position="-1 0.5 1" rotation="0 45 0" width="1" height="1" depth="1" color="#4CC3D9"></a-box>
<a-cylinder position="1 0.75 1" radius="0.5" height="1.5" color="#FFC65D"></a-cylinder>
<a-plane rotation="-90 0 0" width="4" height="4" color="#7BC8A4"></a-plane>
<!-- Sky -->
<a-sky color="#ecfcf4"></a-sky>
</a-scene>

<script>
var scene = document.querySelector('a-scene');
function check () {
Copy link
Contributor

@cvan cvan Apr 3, 2016

Choose a reason for hiding this comment

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

don't want you want to call check on resize of window?
Copy link
Contributor Author

@enjalot enjalot Apr 15, 2016

Choose a reason for hiding this comment

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

I'm adding it, just to confirm that aframe's resize logic doesn't overwrite the user provided canvas dimensions.
var canvas = document.getElementById('mycanvas');
console.log(canvas.width, canvas.height);
Copy link
Contributor

@cvan cvan Apr 1, 2016

Choose a reason for hiding this comment

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

necessary?
}
if (scene.hasLoaded) {
check();
} else {
scene.addEventListener('loaded', check);
}
window.addEventListener('resize', check);
</script>
</body>
</html>
2 changes: 2 additions & 0 deletions src/components/scene/canvas.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ module.exports.Component = register('canvas', {
canvas.classList.add('a-canvas');
canvas.style.height = data.height + '%';
canvas.style.width = data.width + '%';
// Mark canvas as provided/injected by A-Frame.
canvas.dataset.aframeDefault = true;
scene.appendChild(canvas);
}

Expand Down
4 changes: 2 additions & 2 deletions src/core/scene/a-scene.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ var AScene = module.exports = registerElement('a-scene', {
// Possible camera or canvas not injected yet.
if (!camera || !canvas) { return; }

// Update canvas.
if (!isMobile) {
// Update canvas if canvas was provided by A-Frame.
if (!isMobile && canvas.dataset.aframeDefault) {
canvas.style.width = '100%';
canvas.style.height = '100%';
}
Expand Down