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

Adding background-image-smoothing property #2794

Merged

Conversation

arnaudbuy
Copy link
Contributor

This PR adds a background-image-smoothing property, which can be either set to yes (default) or no.

#2775

@maxkfranz maxkfranz self-requested a review December 7, 2020 16:32
Copy link
Member

@maxkfranz maxkfranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. I listed some minor feedback, and after addressing those the PR should be ready to merge.

Thanks for the PR

@@ -1,39 +1,39 @@
{
"build/cytoscape.umd.js": {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you reset this file? It's only for releases. Given the confusion it creates, it might be even better to not generate this file at all any more.

@@ -297,6 +297,7 @@ A background image may be applied to a node's body. The following properties su
* The [`cytoscape-sbgn-stylesheet`](https://github.com/PathwayCommons/cytoscape-sbgn-stylesheet) package serves as a good example for the use of SVG images in a stylesheet. That stylesheet [creates decorations](https://pathwaycommons.github.io/cytoscape-sbgn-stylesheet/) on nodes in line with the [SBGN standard](https://sbgn.github.io).
* **`background-image-crossorigin`**: All images are loaded with a [`crossorigin`](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/img#attr-crossorigin) attribute which may be `anonymous` or `use-credentials`. The default is set to `anonymous`.
* **`background-image-opacity`** : The opacity of the background image.
* **`background-image-smoothing`** : Determines whether background image is smoothed (`yes`, default) or not (`no`). This feature isn't well supported across browsers.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this be made a bit more explicit that it's a only a hint? The browser may or may not respect the value set for this property.

@@ -48,6 +48,7 @@ const styfn = {};
bgFit: { enums: [ 'none', 'contain', 'cover' ], multiple: true },
bgCrossOrigin: { enums: [ 'anonymous', 'use-credentials' ], multiple: true },
bgClip: { enums: [ 'none', 'node' ], multiple: true },
bgSmooth: { enums: [ 'yes', 'no' ], multiple: true },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be named something like bools so that it could be used for other props in future. See bool prop that follows

Reset .size-snapshot.json
Clarified the documentation of the property
Renamed the property type to make it reusable
Formatted the code
@maxkfranz maxkfranz added this to the 3.18.0 milestone Dec 9, 2020
Copy link
Member

@maxkfranz maxkfranz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

@maxkfranz maxkfranz merged commit 6a11822 into cytoscape:unstable Dec 9, 2020
@maxkfranz
Copy link
Member

Merged. Thanks for the PR

@arnaudbuy
Copy link
Contributor Author

Thanks for this merge.

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