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

[@sigma/node-image] Add crossOrigin to TextureManagerOptions #1439

Conversation

TheBigRoomXXL
Copy link

Pull request type

Check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Currently Sigma harcode the cross origin policy of the images rendered by node-image. Because of that it is currently impossible to fetch images that require authentification. I personally had to completly fork the node-image package just to change that one hardcoded attribute.

Issue Number: #1403
Related PR: #1393

What is the new behavior?

A crossOrigin attribute has been added to the TextureManagerOptions . This attribute is then passed to the functions that load images to set the crossOrigin attribute.

Other information

Another PR (#1393 ) is opened for the same feature but it's quite old and precede the creation of the TextureManagerOptions

jacomyal added a commit that referenced this pull request Jul 2, 2024
Details:
- Allows crossOrigin to be `null`, as the comment says it could
- When `null`, then no "crossOrigin" attribute is set (as specified in
  the same comment)
- Runs prettify
@jacomyal
Copy link
Owner

jacomyal commented Jul 2, 2024

I manually rebased and merged this PR, and GitHub didn't catch it. So, I'll close it manually.
I added one commit to make some minor adjustments on your modifications:
93b8370

Thanks for your contribution! It is deployed in @sigma/[email protected].

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.

2 participants