Open wyozi opened 4 years ago
Would you consider passing configurations that match the pptxgenjs SlideMasterProps type here? https://github.com/wyozi/react-pptx/blob/master/src/renderer.ts#L150
const masterSlides = [ { }, // as SlideMasterProps .... ]
And in the initialization of the pptxgenjs normalized.masterSlides.forEach(masterSlide => pptx.defineSlideMaster({masterSlide}));
Then in the slides, adding the masterName prop https://github.com/wyozi/react-pptx/blob/master/src/renderer.ts#L164 const slide = pres.addSlide(slideNode.masterName);
So, that would be the initial approach for the pptxgenjs object. For the preview, we could use the masterSlides as a point of reference for the preview objects as a reference. Is this something you'd be interested as an implementation, @wyozi?
@mckdanoventa the reason why I thought about going with jsx <Slide>
objects was to simplify the overall requirements for the library user. Using SlideMasterProps
requires quite a big context switch from the react-pptx world back to pptxgenjs.
However, the primary motivation for the library is to enable people to use it to solve their problems, so I'm not opposed to a more straightforward prop to solve this in the meantime. What about calling it rawMasterSlides
with SlideMasterProps[]
type?
Ah, I see your point. The caveat with the jsx approach I'm seeing is that the generated ppt's would not have master slides associated to them on the generated ppt. It would only utilize the slide's in the react-pptx
context. Am I making this assumption correctly for the jsx approach?
@mckdanoventa not necessarily actually. I don't see why it wouldn't be possible to convert react-pptx <Image>
, <Text>
etc to corresponding SlideMasterProps#objects
, although I haven't looked that much into it. Additionally, you could have a <MasterSlide>
wrapper for those rather than the <Slide>
one I used in the first post, which would then allow providing other master slide-specific props. That would be the ultimate goal imo.
This might require a relatively large amount of work though, as I'm not sure how much of the current renderer would adapt to the SlideMasterProps#objects
structure.
Busy weeks. I've done the above in a two part approach with the API managing the ppt generation and the react-pptx as a UI render. Not ideal, but gets the job done. I agree, I don't think we need a raw section since it's not the intention of your library.
Do you have an idea of what the scope of the render work would be? It seems like a more ideal solution.
@mckdanoventa no hurries :)
From briefly looking into how pptxgenjs defines the types, it seems like they roughly match the properties that individual objects require when being added to slides. Therefore, I think copying renderSlideObject
in renderer.ts
and slightly changing the implementation to output objects in the right shape should be enough and no massive refactoring would be needed. It does mean there'd be two code paths for master slides and slides, but that sounds okay to me.
Simplified prototype version of this has been added. See index.spec.tsx for an example. Rather than passing masterSlides
directly, it expects master slides to be specified with <MasterSlide>
components on same level as normal slides. I don't know if this is a good idea or not (feedback and ideas welcome).
A proper publicly documented and usable version of this would require deciding on how to pass master slides and then implementing all PptxGenJs SlideMaster primitives (which notably doesn't include all objects that normal slides can use).
@wyozi I would like to attempt adding some support for additional types on the master slide. Also would be great to add in placeholder support: http://gitbrent.github.io/PptxGenJS/docs/masters/#placeholders
@ksk385 that's a good idea! Master slide objects are rendered at https://github.com/wyozi/react-pptx/blob/master/src/renderer.ts#L242 at the moment. Currently master slides can take all the normal slide tags (see https://github.com/wyozi/react-pptx/blob/master/src/index.spec.tsx#L13), almost all of them will just fail at the render phase though.
One approach would be to just extend the supported slide object types in renderer and see if this approach of implementing master slides scales properly. It's possible that placeholders specifically would require a custom node though (something similar to <Text />
, I imagine)
Feel free to ask if there's anything unclear here
I started with a simple improvement, showing the slide backgrounds (color or image) from the master slide.
https://github.com/wyozi/react-pptx/pull/640
Once this is done, I plan to work on introducing images next.
https://gitbrent.github.io/PptxGenJS/docs/masters.html#slide-masters
~This should probably be done by passing a prop to
<Presentation>
with the master slide definitions.~ See https://github.com/wyozi/react-pptx/issues/109#issuecomment-933695622