Skip to content

feature: FillFormat.transparency#140

Open
jonastr wants to merge 1 commit intoscanny:masterfrom
jonastr:shape_alpha
Open

feature: FillFormat.transparency#140
jonastr wants to merge 1 commit intoscanny:masterfrom
jonastr:shape_alpha

Conversation

@jonastr
Copy link
Copy Markdown

@jonastr jonastr commented Dec 9, 2014

example usage:

shape = slide.add_shape(...)
fill = shape.fill
fill.solid()
fill.fore_color.rgb = RGBColor(...)
fill.transparency = 0.5

@scanny scanny changed the title add transparency for fill colors/color format feature: FillFormat.transparency Dec 10, 2014
@scanny
Copy link
Copy Markdown
Owner

scanny commented Dec 10, 2014

Thanks @jonastr, I believe this property should actually be on the fill property and we'll want to call it transparency rather than alpha, just to be consistent with the MS API.

But no need to change the code if it's working for you. If you want to pursue a commit you'll need to provide a feature analysis (proposal) page, an acceptance test, unit tests, and a series of commits consistent with the conventions of the project. Let me know if you're interested in that and I can give you more details. Otherwise we'll keep this on file and use it to inform the implementation when we get to it, before long I'm sure :)

@jonastr
Copy link
Copy Markdown
Author

jonastr commented Dec 10, 2014

@scanny Mhhh... I guess the 'correct' place depends on how you're looking at it. 😄 I put it in ColorFormat to reflect the structure of the XML document and also named the property accordingly (and also it appeared to be the easiest way for me as to reduce the programming/code understanding effort). In the XML document, it is a property of the color - which, well, yeah, is a bit strange since transparency commonly is not a concern of the color space. 😄 I thus assume that, when you say "MS API", you refer to the VBA/Whatever components? Then, yeah, it might really make more sense to put the property somewhere else.

Other than that... I was expecting some more constraints before this feature can be merged. :) Unfortunately, I can't invest more time to make the code/commit compliant to your requirements. Therefore, I'll stick with my implementation for now. Of course, please feel free to improve as needed and copy to the official solution whatever you think makes sense.

@scanny
Copy link
Copy Markdown
Owner

scanny commented Dec 10, 2014

Of course, as you say, there will always be a certain amount of arbitrariness in the design decisions. And I'm not entirely sure we won't end up attaching it to color after all, depending on the analysis of what the behavior is of PowerPoint itself when you set the .transparency property. I vaguely suspect the reason they put it onto FillFormat is so both the foreground and background alpha are set at once for a pattern fill. It could also be that it is implemented differently for a blipFill (picture fill). That's probably the main reason why the analysis/proposal page has evolved to be the best first step in a feature add process; to reduce rework in the tests and implementation.

Grateful for the pull request, these come in handy when implementing the general case :)

@hoopes hoopes mentioned this pull request Mar 24, 2015
@ghost
Copy link
Copy Markdown

ghost commented Sep 23, 2016

Produced my own fix before coming across these pull requests. I would be interested to put together feature analysis, look at the 2 contributions, compare them to my own implementation and see if we can formalize this feature. Would be good to get some more details on what are the requirements for the contribution to be accepted.

@davidperumalchang
Copy link
Copy Markdown

Hey @scanny , is there any plan to include this PR to add support for FillFormat.transparency 🙏

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants