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

Add result parameters to gradient, pattern creation methods. #46

Merged
merged 5 commits into from Dec 18, 2017

Conversation

Projects
None yet
4 participants
@dmitshur
Collaborator

dmitshur commented Oct 20, 2017

This change allows the result to be used.

Create wrappers for CanvasGradient, CanvasPattern types.

Example usage (that I tested with):

var canvas = document.GetElementByID("canvas").(*dom.HTMLCanvasElement)
var ctx = canvas.GetContext2d()

gradient := ctx.CreateRadialGradient(0, 0, 8*1.75, 0, 0, 0)
gradient.AddColorStop(0, "rgba(0, 0, 0, 0)")
gradient.AddColorStop(1, "rgba(0, 0, 0, 0.3)")
ctx.Set("fillStyle", gradient)
ctx.Ellipse(0, 0, 8*1.75, 8*1.75, 0, 0, Tau, false)
ctx.Fill()

Updates #44. /cc @luckcolors

References:

@dmitshur dmitshur requested a review from dominikh Oct 20, 2017

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 1, 2017

Collaborator

Ping @dominikh.

Collaborator

dmitshur commented Nov 1, 2017

Ping @dominikh.

@dominikh

This comment has been minimized.

Show comment
Hide comment
@dominikh

dominikh Nov 9, 2017

Owner

If we start copying documentation from the MDN we'll have to look into the license. We'd also have to address terminology such as raises, as Go doesn't use exceptions.

Owner

dominikh commented Nov 9, 2017

If we start copying documentation from the MDN we'll have to look into the license. We'd also have to address terminology such as raises, as Go doesn't use exceptions.

@luckcolors

This comment has been minimized.

Show comment
Hide comment
@luckcolors

luckcolors Nov 9, 2017

Contributor

About the license, MDN is very permissive https://developer.mozilla.org/en-US/docs/MDN/About#Copyrights_and_licenses.
We just have to put a link to the page of the doc.
Do you want me to make a pull request so we can fix this in all the other places?

Contributor

luckcolors commented Nov 9, 2017

About the license, MDN is very permissive https://developer.mozilla.org/en-US/docs/MDN/About#Copyrights_and_licenses.
We just have to put a link to the page of the doc.
Do you want me to make a pull request so we can fix this in all the other places?

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 9, 2017

Collaborator

If we start copying documentation from the MDN we'll have to look into the license.

/cc @slimsag As I understand, you've had to deal with this in vecty documentation as well. Do you have any recommendations for how to proceed?

Just to clarify, I didn't copy the documentation outright, I used MDN as a reference. Some parts of it were copied.

Collaborator

dmitshur commented Nov 9, 2017

If we start copying documentation from the MDN we'll have to look into the license.

/cc @slimsag As I understand, you've had to deal with this in vecty documentation as well. Do you have any recommendations for how to proceed?

Just to clarify, I didn't copy the documentation outright, I used MDN as a reference. Some parts of it were copied.

Fix lint issue in CreateRadialGradient doc
This was an accidental oversight.
@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 9, 2017

Collaborator

Do you want me to make a pull request so we can fix this in all the other places?

Let's hold off on that for now. It would be a lot of work to make such a change and to review it, with questionable benefits. We can consider it later (feel free to open an issue about for further discussion).

Collaborator

dmitshur commented Nov 9, 2017

Do you want me to make a pull request so we can fix this in all the other places?

Let's hold off on that for now. It would be a lot of work to make such a change and to review it, with questionable benefits. We can consider it later (feel free to open an issue about for further discussion).

Rephrase AddColorStop doc to say it panics on invalid input
"raised" is a JavaScript term that doesn't apply here.

Instead, document that a Go panic happens, and its type is a pointer
to https://godoc.org/github.com/gopherjs/gopherjs/js#Error.
@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 9, 2017

Collaborator

@dominikh I've fixed the "raised" terminology issue in 97fdc88.

Collaborator

dmitshur commented Nov 9, 2017

@dominikh I've fixed the "raised" terminology issue in 97fdc88.

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Nov 9, 2017

Collaborator

For reference, I tested with:

package main

import (
	"fmt"

	"github.com/gopherjs/gopherjs/js"
	"honnef.co/go/js/dom"
)

func main() {
	var document = dom.GetWindow().Document()
	var canvas = document.CreateElement("canvas").(*dom.HTMLCanvasElement)
	var ctx = canvas.GetContext2d()
	gradient := ctx.CreateRadialGradient(0, 0, 10, 0, 0, 0)
	defer func() {
		e := recover()
		if e == nil {
			fmt.Println("no panic")
			return
		}
		err, ok := e.(*js.Error)
		if !ok {
			fmt.Println("recovered value type is not *js.Error")
			return
		}
		fmt.Println("recovered a *js.Error:")
		fmt.Println(err)
		fmt.Println(err.Stack())
	}()
	gradient.AddColorStop(1.1, "red")
}

Which resulted in:

image

Collaborator

dmitshur commented Nov 9, 2017

For reference, I tested with:

package main

import (
	"fmt"

	"github.com/gopherjs/gopherjs/js"
	"honnef.co/go/js/dom"
)

func main() {
	var document = dom.GetWindow().Document()
	var canvas = document.CreateElement("canvas").(*dom.HTMLCanvasElement)
	var ctx = canvas.GetContext2d()
	gradient := ctx.CreateRadialGradient(0, 0, 10, 0, 0, 0)
	defer func() {
		e := recover()
		if e == nil {
			fmt.Println("no panic")
			return
		}
		err, ok := e.(*js.Error)
		if !ok {
			fmt.Println("recovered value type is not *js.Error")
			return
		}
		fmt.Println("recovered a *js.Error:")
		fmt.Println(err)
		fmt.Println(err.Stack())
	}()
	gradient.AddColorStop(1.1, "red")
}

Which resulted in:

image

@slimsag

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Nov 9, 2017

/cc @slimsag As I understand, you've had to deal with this in vecty documentation as well. Do you have any recommendations for how to proceed?

Correct. MDN documentation is licensed as CC-BY-SA 2.5 (which you can read about here). We use MDN documentation in generated Vecty packages, for example https://godoc.org/github.com/gopherjs/vecty/elem

There are two main things to consider:

  1. You must provide attribution and a link to the license. Because these packages are also visible on godoc.org, I suggest placing such attribution in the package docs so it is visible from there too. e.g. the above link shows this in our package docs:

Generated from "HTML element reference" by Mozilla Contributors, https://developer.mozilla.org/en-US/docs/Web/HTML/Element, licensed under CC-BY-SA 2.5.

  1. If you modify/alter/build upon their documentation, you must release it under the same license. This is as simple as saying "Documentation strings in this package are licensed under CC-BY-SA 2.5" (you can of course continue to license the code under any license you prefer, only the docstrings are affected).

Just to clarify, I didn't copy the documentation outright, I used MDN as a reference. Some parts of it were copied.

  • If some parts were copied and/or modified, those copied parts are CC-BY-SA 2.5.
  • If you only previously looked at the MDN, and had unique thoughts about how to write the documentation on your own, those are under any license you choose.

Disclaimer: I am not a lawyer, this isn't legal advice, yada yada. Check with the actual license (https://creativecommons.org/licenses/by-sa/2.5/) to be sure. 😄

slimsag commented Nov 9, 2017

/cc @slimsag As I understand, you've had to deal with this in vecty documentation as well. Do you have any recommendations for how to proceed?

Correct. MDN documentation is licensed as CC-BY-SA 2.5 (which you can read about here). We use MDN documentation in generated Vecty packages, for example https://godoc.org/github.com/gopherjs/vecty/elem

There are two main things to consider:

  1. You must provide attribution and a link to the license. Because these packages are also visible on godoc.org, I suggest placing such attribution in the package docs so it is visible from there too. e.g. the above link shows this in our package docs:

Generated from "HTML element reference" by Mozilla Contributors, https://developer.mozilla.org/en-US/docs/Web/HTML/Element, licensed under CC-BY-SA 2.5.

  1. If you modify/alter/build upon their documentation, you must release it under the same license. This is as simple as saying "Documentation strings in this package are licensed under CC-BY-SA 2.5" (you can of course continue to license the code under any license you prefer, only the docstrings are affected).

Just to clarify, I didn't copy the documentation outright, I used MDN as a reference. Some parts of it were copied.

  • If some parts were copied and/or modified, those copied parts are CC-BY-SA 2.5.
  • If you only previously looked at the MDN, and had unique thoughts about how to write the documentation on your own, those are under any license you choose.

Disclaimer: I am not a lawyer, this isn't legal advice, yada yada. Check with the actual license (https://creativecommons.org/licenses/by-sa/2.5/) to be sure. 😄

Provide attribution for documentation
This text was created after consulting the "Using MDN Web Docs content"
section at https://developer.mozilla.org/en-US/docs/MDN/About#Copyrights_and_licenses.
@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Dec 15, 2017

Collaborator

Thank you very much @slimsag.

@dominikh I believe I've addressed the remaining task with ff5f7d3 and this PR should be ready to be merged now. PTAL.

Collaborator

dmitshur commented Dec 15, 2017

Thank you very much @slimsag.

@dominikh I believe I've addressed the remaining task with ff5f7d3 and this PR should be ready to be merged now. PTAL.

@dmitshur dmitshur merged commit 2366b08 into master Dec 18, 2017

@dmitshur dmitshur deleted the gradient-pattern branch Dec 18, 2017

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