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

GH 821: PowerPoint Export #1139

Merged
merged 22 commits into from
Apr 10, 2023
Merged

GH 821: PowerPoint Export #1139

merged 22 commits into from
Apr 10, 2023

Conversation

ejulio-ts
Copy link
Contributor

@ejulio-ts ejulio-ts commented Apr 6, 2023

This fixes #821

PowerPoint (.pptx) files are just a bunch of XML files following the Office Open XML (OOXML) standard and then zipped.
This adds a pptx lib that wraps all the logic to create these files around a Presentation. There's a base template with the files that don't change and then the slides and some other required metadata are generated on the fly. The XML content was based on the content of an exported presentation. The intent here wasn't to create pptx library.

Important in pptx/pptx.go I tested 3 different approaches to generate the XML files.

  • long template string (getRelsSlideXml)
  • string builder with long templates (getPresentationXmlRels)
  • string builder with a single tag per line (getAppXml)

I don't have any strong preference. The first one is quite complicated to update, as finding the template tags can be hard. The last one spans over many lines with long functions. So, open for ideas here and then I can use a single standard after this review. Just made it this way to see if there was some preference to keep.

Tested the resulting `.pptx. file on:

  • MS PowerPoint on Mac and Windows
  • Open Office on Ubuntu
  • KeyNote on Mac
  • OneDrive
  • Google Drive

ToDo:

  • add links and appendix: this can be a separate PR with just linking, to keep this one small..

Below is the result for

mobile -> backend

backend: {
    link: layers.backend
}

mobile: {
    link: layers.mobile
}

layers: {
    backend: {
        api -> mongodb
        worker <-> kafka
        worker -> mongodb

        kafka.shape: queue
        mongodb.shape: cylinder
    }
    mobile: {
        a button
        b button
        c button
        steps: {
            1: {
                user clicks -> a button
            }
            2: {
                a button -> authorized?
                authorized? -> return: no    
            }
            3: {
                authorized? -> save data: yes
            }
        }
    }
}

scenarios: {
    offline: {
        backend.style.opacity: 0.1
        mobile -> local db
        local db.shape: cylinder
        mobile -> backend: try to reconnect every 1s
    }
}

app-flow.pptx

About the metadata, here's a screenshot
2023-04-06_10-51

@ejulio-ts ejulio-ts marked this pull request as draft April 6, 2023 15:12
Copy link
Contributor

@gavin-ts gavin-ts left a comment

Choose a reason for hiding this comment

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

I think the builder option makes sense, it doesn't need to be long templates, or a single tag each, just what makes sense for each call. builder is also used for svg rendering and the giant template option is hard to use

d2cli/main.go Outdated Show resolved Hide resolved
d2cli/main.go Show resolved Hide resolved
@ejulio-ts ejulio-ts marked this pull request as ready for review April 6, 2023 18:17
@ejulio-ts
Copy link
Contributor Author

I think the builder option makes sense, it doesn't need to be long templates, or a single tag each, just what makes sense for each call. builder is also used for svg rendering and the giant template option is hard to use

Yeah, I prefer the builder for sure, just not sure about what style. So leaving open for a discussion

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

why is it so small?

Kapture.2023-04-06.at.13.07.10.mp4

@ejulio-ts
Copy link
Contributor Author

Let me check it..
Also, just checked that adding the links doesn't seem that hard, so I'll add it here..
Finally, I was talking to @gavin-ts and I'll see to use sections to group layers/scenarios/steps as in https://support.microsoft.com/en-us/office/organize-your-powerpoint-slides-into-sections-de4bf162-e9cc-4f58-b64a-7ab09443b9f8

@alixander
Copy link
Collaborator

oh what in the fuck, cool lol

@ejulio-ts
Copy link
Contributor Author

About the image size, it was an issue with integer division in IMAGE_WIDTH. I changed it to a hard coded constant instead of calculating it there.
Also, it had an issue with resizing that was only considering the largest side, but not the aspect ratio that would result in a better fit.

@ejulio-ts
Copy link
Contributor Author

It turns out that adding the links is a bit more complicated than I expected.
From what I can tell I'm missing some tag/property and the link is not showing up in Keynote/PowerPoint.

So, I don't want to keep this one hanging because of links/sections as they can be separate PRs. Can you review this one then while I keep looking that?

d2cli/main.go Outdated Show resolved Hide resolved
d2cli/main.go Outdated Show resolved Hide resolved
lib/pptx/presentation.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

didn't review much of the voodoo going on in the construction, mostly just wanna make sure the interfacing with cli and tests are good

d2cli/main.go Outdated Show resolved Hide resolved
d2cli/main.go Outdated Show resolved Hide resolved
lib/pptx/pptx.go Show resolved Hide resolved
lib/pptx/pptx.go Outdated Show resolved Hide resolved
lib/pptx/pptx.go Outdated Show resolved Hide resolved
lib/pptx/presentation.go Outdated Show resolved Hide resolved
lib/pptx/pptx.go Outdated Show resolved Hide resolved
@ejulio-ts
Copy link
Contributor Author

  • Changed to use templates instead of raw XML strings

CI is failing with playwright (not sure if I'm missing something)
image

@alixander
Copy link
Collaborator

@ejulio-ts nah, it's a known issue, just add a skip_ci like we do for pdfs.

Copy link
Collaborator

@alixander alixander left a comment

Choose a reason for hiding this comment

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

add changelog plz

@ejulio-ts ejulio-ts merged commit b84095b into terrastruct:master Apr 10, 2023
@ejulio-ts ejulio-ts deleted the gh-821-ppt branch April 10, 2023 20:21
@ejulio-ts ejulio-ts mentioned this pull request Apr 10, 2023
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.

investigate Powerpoint export
3 participants