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

If doc.body is null, throw "move elm.js <script> " #709

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@devinrhode2

devinrhode2 commented Sep 11, 2016

Checkout the last few commits on my project here: https://github.com/devinrhode2/structured-elm-todomvc/tree/emberify2
If you rewind and replay the commits you will see logic behind my error message.

If doc.body is null, throw "move elm.js <script> "
Checkout the last few commits on my project here: https://github.com/devinrhode2/structured-elm-todomvc/tree/emberify2
If you rewind and replay the commits you will see logic behind my error message.
@process-bot

This comment has been minimized.

Show comment
Hide comment
@process-bot

process-bot Sep 11, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

process-bot commented Sep 11, 2016

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@devinrhode2

This comment has been minimized.

Show comment
Hide comment
@devinrhode2

devinrhode2 Sep 11, 2016

haskell platform was taking a while to install, so I figured I'd just search for and directly edit the js here using githubs online editor. I have not managed to build and run this yet.

devinrhode2 commented Sep 11, 2016

haskell platform was taking a while to install, so I figured I'd just search for and directly edit the js here using githubs online editor. I have not managed to build and run this yet.

@devinrhode2

This comment has been minimized.

Show comment
Hide comment

devinrhode2 commented Sep 11, 2016

evenwithsomethinginlocalstorage

@lukewestby

This comment has been minimized.

Show comment
Hide comment
@lukewestby

lukewestby Sep 12, 2016

Member

If the document isn't finished loading then Module.embed() is going to throw for the same reason, so handling this will have to be moved one level higher than fullscreen(). I wonder if we could hide this detail from users altogether by wrapping startup with

function startElmProgram()
{
  // ...
}

if (document.readyState === 'complete')
{
  startElmProgram();
}
else
{
  document.addEventListener('DOMContentLoaded', startElmProgram);
}
Member

lukewestby commented Sep 12, 2016

If the document isn't finished loading then Module.embed() is going to throw for the same reason, so handling this will have to be moved one level higher than fullscreen(). I wonder if we could hide this detail from users altogether by wrapping startup with

function startElmProgram()
{
  // ...
}

if (document.readyState === 'complete')
{
  startElmProgram();
}
else
{
  document.addEventListener('DOMContentLoaded', startElmProgram);
}
@devinrhode2

This comment has been minimized.

Show comment
Hide comment
@devinrhode2

devinrhode2 Sep 12, 2016

No. That's way less robust. Elm should not mess with setting up a
document.ready event here in my opinion. If you really want to do that,
copy out the document.ready our of the latest version of jQuery. I don't
think this code structure would work well with that. There's probably a
handful of other good reasons to put scripts at the bottom.

We should, however, hide this from users by creating a proper index.html
file for users. I copied out of few good but missing tags from embers
index.html, however, there's no compile time hook to add content to the
head tag. (will share some links in a sec)

On 11 Sep 2016 17:59, "Luke Westby" notifications@github.com wrote:

I wonder if we could hide this detail from users by wrapping startup with

function startElmProgram()
{
// ...
}
if (document.readyState === 'complete')
{
startElmProgram();
}else
{
document.addEventListener('DOMContentLoaded', startElmProgram);
}


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/elm-lang/core/pull/709#issuecomment-246217154, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAg8qCahQO7Im9PbiH5jmmi1IbqWkUDSks5qpKPogaJpZM4J6IwM
.

devinrhode2 commented Sep 12, 2016

No. That's way less robust. Elm should not mess with setting up a
document.ready event here in my opinion. If you really want to do that,
copy out the document.ready our of the latest version of jQuery. I don't
think this code structure would work well with that. There's probably a
handful of other good reasons to put scripts at the bottom.

We should, however, hide this from users by creating a proper index.html
file for users. I copied out of few good but missing tags from embers
index.html, however, there's no compile time hook to add content to the
head tag. (will share some links in a sec)

On 11 Sep 2016 17:59, "Luke Westby" notifications@github.com wrote:

I wonder if we could hide this detail from users by wrapping startup with

function startElmProgram()
{
// ...
}
if (document.readyState === 'complete')
{
startElmProgram();
}else
{
document.addEventListener('DOMContentLoaded', startElmProgram);
}


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/elm-lang/core/pull/709#issuecomment-246217154, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAg8qCahQO7Im9PbiH5jmmi1IbqWkUDSks5qpKPogaJpZM4J6IwM
.

@devinrhode2

This comment has been minimized.

Show comment
Hide comment
@devinrhode2

devinrhode2 Sep 12, 2016

So this is the default index.html template for a new ember app:
https://github.com/rwjblue/--new-app-blueprint/blob/modules/src/ui/index.html

Here's a real world index.html that is filled out more (obviously not all
this is quite as important as the things in the default template):
https://github.com/rwjblue/--ghost-modules-sample/blob/grouped-collections/src/ui/index.html
However... I'm probably incorrect. I think ultimately elm shouldn't be
trying to work with the dom until it's ready. I don't know anything about
this code base and architecture. However, I don't think the whole app
startup should wait for document.ready. It should simply be the rendering
layer. This is why I intuitively and quickly responded negatively to your
suggestion

On 11 Sep 2016 18:26, "devin rhode" devinrhode2@gmail.com wrote:

No. That's way less robust. Elm should not fuck with setting up a
document.ready event here in my opinion. If you really want to do that,
copy out the document.ready our of the latest version of jQuery. I don't
think this code structure would work well with that. There's probably a
handful of other good reasons to put scripts at the bottom.

We should, however, hide this from users by creating a proper index.html
file for users. I copied out of few good but missing tags from embers
index.html, however, there's no compile time hook to add content to the
head tag. (will share some links in a sec)

On 11 Sep 2016 17:59, "Luke Westby" notifications@github.com wrote:

I wonder if we could hide this detail from users by wrapping startup with

function startElmProgram()
{
// ...
}
if (document.readyState === 'complete')
{
startElmProgram();
}else
{
document.addEventListener('DOMContentLoaded', startElmProgram);
}


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/elm-lang/core/pull/709#issuecomment-246217154, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAg8qCahQO7Im9PbiH5jmmi1IbqWkUDSks5qpKPogaJpZM4J6IwM
.

devinrhode2 commented Sep 12, 2016

So this is the default index.html template for a new ember app:
https://github.com/rwjblue/--new-app-blueprint/blob/modules/src/ui/index.html

Here's a real world index.html that is filled out more (obviously not all
this is quite as important as the things in the default template):
https://github.com/rwjblue/--ghost-modules-sample/blob/grouped-collections/src/ui/index.html
However... I'm probably incorrect. I think ultimately elm shouldn't be
trying to work with the dom until it's ready. I don't know anything about
this code base and architecture. However, I don't think the whole app
startup should wait for document.ready. It should simply be the rendering
layer. This is why I intuitively and quickly responded negatively to your
suggestion

On 11 Sep 2016 18:26, "devin rhode" devinrhode2@gmail.com wrote:

No. That's way less robust. Elm should not fuck with setting up a
document.ready event here in my opinion. If you really want to do that,
copy out the document.ready our of the latest version of jQuery. I don't
think this code structure would work well with that. There's probably a
handful of other good reasons to put scripts at the bottom.

We should, however, hide this from users by creating a proper index.html
file for users. I copied out of few good but missing tags from embers
index.html, however, there's no compile time hook to add content to the
head tag. (will share some links in a sec)

On 11 Sep 2016 17:59, "Luke Westby" notifications@github.com wrote:

I wonder if we could hide this detail from users by wrapping startup with

function startElmProgram()
{
// ...
}
if (document.readyState === 'complete')
{
startElmProgram();
}else
{
document.addEventListener('DOMContentLoaded', startElmProgram);
}


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/elm-lang/core/pull/709#issuecomment-246217154, or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAg8qCahQO7Im9PbiH5jmmi1IbqWkUDSks5qpKPogaJpZM4J6IwM
.

@rtfeldman

This comment has been minimized.

Show comment
Hide comment
@rtfeldman

rtfeldman Sep 12, 2016

Member

No. That's way less robust. Elm should not fuck with

Let's keep it civil. :)

@devinrhode2 I like the general UX idea here, but have you seen this? https://github.com/elm-lang/core/pull/709#issuecomment-246212555

Member

rtfeldman commented Sep 12, 2016

No. That's way less robust. Elm should not fuck with

Let's keep it civil. :)

@devinrhode2 I like the general UX idea here, but have you seen this? https://github.com/elm-lang/core/pull/709#issuecomment-246212555

@devinrhode2

This comment has been minimized.

Show comment
Hide comment

devinrhode2 commented Sep 12, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment