Confusing behavior for `downloadURL` and `embedCode` both specified

[adamschwartz]

From the source:

  get copyText() {
    if (this.downloadURL) return `<script src="${this.downloadURL}"></script>`

    return this.config.embedCode || this.store.embedCode
  }

This is resulting in this for Weebly (on the homepage):

I’m thinking that maybe the logic here is backwards? Like if an embedCode is provided, use that, and only if there is no embedCode but there is a downloadURL should we revert to the concatenation?

Also, in the concatenation, we should probably strip the protocol, right? http://embed.books.eager.io/script-tag.html#https

cc @zackbloom

[GirlBossRush]

@adamschwartz, it's an interesting question. I think the hero demo is configured oddly.

We briefly chatted about what's the expected behavior when only a downloadURL is configured. My feeling is that a the config wide downloadURL is odd fitting since plugins are unique to targets. It should be scoped to the particular target. We already support scoped target configs overriding the global config 🐳

Our most basic config:

new EmbedBox({
  embedCode: "<script src='http://example.com/foo.js</script>"
})

This example would only show targets that support an embedCode (all of them?)

A plugin config:

new EmbedBox({
  targets: {
    wordpress: {
      downloadURL: "http://example.com/wordpress.zip"
    }
  }
})

This example would only show a single target, WordPress.

This also brings up our visibleTargets property. I'm leaning toward it being confusing. visibleTargets does double duty of visibility and ordering. @zackbloom suggested a priority option awhile back (now order, see https://github.com/EagerIO/EmbedBox/issues/56#issuecomment-239594289). I think it'd be uncommon for a developer to hide a particular target, but if needed that can be arranged.

new EmbedBox({
  embedCode: "<script src='http://example.com/foo.js</script>",
  targets: {
    wordpress: {
      downloadURL: "http://example.com/wordpress.zip",
      priority: 1
    },
    joomla: {
      downloadURL: "http://example.com/joomla.zip",
      priority: 2
    },
    wix: {
      visible: false
    }
  }
})

This config would show all targets except Wix. WordPress and Joomla would appear at the top of the list, using a downloadURL instead of the embedCode.

Thoughts?

[adamschwartz]

So the idea then would be to remove the global downloadURL option which the demo currently uses?

[GirlBossRush]

Yep. Given how odd it behaves, I'd say it makes sense.

[adamschwartz]

So copyText would change to this?

  get copyText() {
    return this.config.embedCode || this.store.embedCode
  }

[GirlBossRush]

Probably not for the usual flow. I think that targets with downloadURLs will either be plugin oriented where it's about the software specific flow of uploading some file.

A modified version might make sense for the generic target since it can have a combination downloadURL and script to copy.

[GirlBossRush]

Progress update:

The latest sorting logic is up and should be much easier to understand since it takes after the install.json order concept. http://embedbox.io/#ordering