Would you accept PRs for : about pptx-automizer

[singerla]

Hi! I'm happy you like it 😃

Regarding your suggested improvements:

1.) I did also encounter problems with older ppt files in the past. My way to fix it was to copy & paste all parts and force ppt 2019 to add the creationIds. Booh! 😃

The primary intention for getCreationIds was to become independent of shape names and slide numbers, because a ppt user can easily change them. But I agree, it should fail more gracefully, maybe it should'n fail at all, but skip the item.

2.) If all you want is to retreive a list of all slides, you could also use (brand new) XmlRelationshipHelper

  const pres = await automizer
    .loadRoot(`EmptyTemplate.pptx`)
    .load('SlideMasters.pptx');

  const archive = pres.getTemplate('SlideMasters.pptx').archive;
  const allSlides = (await new XmlRelationshipHelper().initialize(
    archive,
    `presentation.xml.rels`,
    `ppt/_rels`,
    'slides/slide',
  )) as Target[];

  // You will need to sort this array by slide.number
  allSlides.forEach((slide) => {
    console.log(slide.number);
  });

3.) You might at first want to take a look at the recently introduced basic class "HasShapes", especially at the FindElementStrategy type used in findElementOnSlide method. It seems to me as if your idea will fit perfect as a third mode, like mode: 'findByElementCreationId' | 'findByElementName' | 'findByElementId';. I was already trying to make it more flexible to find a shape on a slide, see FindElementSelector type in src/types/types.ts.

It will be very welcome to implement your suggested point 3.) not only as a "helper", but from bottom to top.

Thanks in advance for any coming PR! 😃

[MP70]

2.) If all you want is to retreive a list of all slides, you could also use (brand new) XmlRelationshipHelper

  const pres = await automizer
    .loadRoot(`EmptyTemplate.pptx`)
    .load('SlideMasters.pptx');

  const archive = pres.getTemplate('SlideMasters.pptx').archive;
  const allSlides = (await new XmlRelationshipHelper().initialize(
    archive,
    `presentation.xml.rels`,
    `ppt/_rels`,
    'slides/slide',
  )) as Target[];

  // You will need to sort this array by slide.number
  allSlides.forEach((slide) => {
    console.log(slide.number);
  });

Awesome, thanks, I didn't see that, possibly because it wasn't written when I first started using this! Would you like me to turn that into a tiny little helper, or is it good as is?

3.) You might at first want to take a look at the recently introduced basic class "HasShapes", especially at the FindElementStrategy type used in findElementOnSlide method. It seems to me as if your idea will fit perfect as a third mode, like mode: 'findByElementCreationId' | 'findByElementName' | 'findByElementId';. I was already trying to make it more flexible to find a shape on a slide, see FindElementSelector type in src/types/types.ts.

Thanks for the pointer, that makes sense. Maybe something like

else if (selector.slideNumber) {
  strategies.push({
    mode: 'findAllTextElementsBySlideNumber',
    selector: selector.slideNumber,
  });

class XmlHelper {
  // ...

  static async findAllTextElementsBySlideNumber(archive: IArchive, slideNumber: number): Promise<XmlDocument[]> {
    const slideRelFile = `slides/slide${slideNumber}.xml`;
    const slideXml = await this.getXmlFromArchive(archive, 'ppt/' + slideRelFile);

    if (!slideXml) {
      throw new Error(`Slide with number ${slideNumber} not found.`);
    }

    const shapeTree = slideXml.getElementsByTagName('p:spTree').item(0);
    const shapeNodes = shapeTree.getElementsByTagName('p:sp');
    const textElements: XmlDocument[] = [];

    for (let i = 0; i < shapeNodes.length; i++) {
      const shapeNode = shapeNodes.item(i);
      const txBody = shapeNode.getElementsByTagName('p:txBody').item(0);

      if (txBody) {
        textElements.push(shapeNode);
      }
    }

    return textElements;
  }

  // ...
}

Would you (or the user) have any need for anything similar for other tags, i.e is it preferable to make a broader findElementsBySlide(tagTypes?:string[]) for any reason? From what I can think of text replace is the only real use case for this but maybe I'm wrong and the user might want to edit all charts or something?

It will be very welcome to implement your suggested point 3.) not only as a "helper", but from bottom to top.

I don't disagree, though the helper will get the user a simple forEach/loop away. I'll take a look later :)

[singerla]

2. It would be great to have this as a helper! Feel free to add it to xml-template-helper.ts!

What about referencing the helper function later to be callable from any loaded template?

const allSlidesSorted = pres.getTemplate('SlideMasters.pptx').getAllSlideNumbers()

I guess this should require only a few changes in src/classes/template.ts and the implemented src/interfaces/itemplate.ts. Imagine to add e.g. getAllSlideNumbers: () => Promise<number[]>; to the latter.

3. Maybe we could introduce a new class of helpers for this and do the basic stuff later. What about xml-slide-helper.ts to offer findAllTextElementsBySlideNumber and whatever may be helpful in the future?

I could also imagine to reference this from slide.ts, so it can be easily accessed like this:

  await pres
    .addSlide('MyTemplate.pptx', 1, (slide) => {
      // this could reference e.g. XmlSlideHelper.findAllTextElementsBySlideNumber
      // for the current slide
      const myTextShapes = slide.findAllTextElements();
    })

Thanks in advance and have fun! 😄

[MP70]

2. It would be great to have this as a helper! Feel free to add it to xml-template-helper.ts!

What about referencing the helper function later to be callable from any loaded template?

const allSlidesSorted = pres.getTemplate('SlideMasters.pptx').getAllSlideNumbers()

I guess this should require only a few changes in src/classes/template.ts and the implemented src/interfaces/itemplate.ts. Imagine to add e.g. getAllSlideNumbers: () => Promise<number[]>; to the latter.

Awesome, have quickly done this.

3. Maybe we could introduce a new class of helpers for this and do the basic stuff later. What about xml-slide-helper.ts to offer findAllTextElementsBySlideNumber and whatever may be helpful in the future?

I personally think this is the only one I'll need to add in myself, but will implement like that :)

I could also imagine to reference this from slide.ts, so it can be easily accessed like this:

  await pres
    .addSlide('MyTemplate.pptx', 1, (slide) => {
      // this could reference e.g. XmlSlideHelper.findAllTextElementsBySlideNumber
      // for the current slide
      const myTextShapes = slide.findAllTextElements();
    })

Thanks in advance and have fun! 😄

That's exactly what I was planning, so if you are happy with the No.2 PR then I'll write this next.

[singerla]

Thank you so much for solving 2.), it was merged as #59 .
I would be very happy if you could do the same regarding 3.). 🚀

[MP70]

Morning :)

Just submitted a working PR #61 but I haven't 100% wrapped my head around creationIds yet.. I don't use them at all. Please could I ask you to review my creationIds implementation let me know what needs to change if anything?

[MP70]

PR #61 merged