-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
fix(BlendImage): Update Docs #9876
fix(BlendImage): Update Docs #9876
Conversation
- correct JSDoc comment for BlendImage filter's 'image' property - added JSDoc comments for BlendImage filter's 'mode' property - removed 'alpha' from BlendImage filter example in two places. Since the alpha property is not supported, it shouldn't be in the example.
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
* multiply the values of each channel (R, G, B, and A) of the filter image by | ||
* their corresponding values in the base image. 'mask' will only look at the | ||
* alpha channel of the filter image, and apply those values to the base | ||
* image's alpha channel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I m not 100% sure this is the best description, but if you change it is probably mirroring the reality at the moment.
I think we should link to mdn pages that explain what multiply or mask would do and then stick our implementation to those.
* }); | ||
* object.filters.push(filter); | ||
* object.applyFilters(); | ||
* canvas.renderAll(); | ||
*/ | ||
export class BlendImage extends BaseFilter { | ||
/** | ||
* Color to make the blend operation with. default to a reddish color since black or white | ||
* gives always strong result. | ||
* Image to make the blend operation with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would just call this FabricImage instead of Image, and i would specify that FabricImage transform properties are taken in consideration
Thank you @DanKayeEvolveLab. Could you have a look at my comments and address what is possible? |
@asturur Thank you for finishing up these docs updates! My apologies, I got a bit distracted and forgot to circle back to your feedback. |
Description
Updating docs for the BlendImage filter, after finding inconsistencies in Issue #5797