Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Margin not working with placeholder text (v3) #640

Closed
bestis opened this issue Jan 7, 2020 · 7 comments
Closed

Margin not working with placeholder text (v3) #640

bestis opened this issue Jan 7, 2020 · 7 comments
Assignees
Milestone

Comments

@bestis
Copy link

bestis commented Jan 7, 2020

Hi,

This used to work:
defineSlideMaster having object:

{
	placeholder: {
		options: {
			name: "title",
			type: "title",
			x: leftMargin,
			y: 0.2,
			w: layoutWidth - leftMargin - logoWidth,
			h: 0.3,
			bold: true,
			fontFace: "Helvetica",
			color: "404040",
			fontSize: 18,
			align: "left",
			valign: "middle",
			margin: 0,
		},
		text: "",
	},
},

And then adding to it.

imageSlide.addText(slideTitle, {
	placeholder: "title",
});

Some screenshots to visualize the problem:

Old version:
title-has-no-margin-v2
New version:
title-has-margin-v3
And settings:
title-has-margins-v3

Tested with https://github.com/gitbrent/PptxGenJS.git#2cee3f0c412106aefb1f87fc16bfef78ae5521a7 (as it solved the JSZip problem).

@bestis
Copy link
Author

bestis commented Jan 7, 2020

Oh and just noticed that the "Vertical alignment" is not working either. As we can see on the screenshot it's "Top", but I did specify it as "middle".

@GodsBest
Copy link

Yes, I can confirm too that “vertical alignment” is not working. The default is top; setting valign=“bottom” or valign=“middle” does not change the alignment.

@ReimaFrgos
Copy link
Contributor

ReimaFrgos commented Jul 7, 2020

@gitbrent looks like there's a logic issue infunction genXmlBodyProperties(slideObject)

The first if statement block sets the anchor attribute for text objects to control vertical alignment, but Placeholders are being sent to the else block which doesn't set the anchor in bodyProperties.

This would also affect the margins as they are set in the same block of code

@eddy-lau
Copy link

eddy-lau commented Jan 5, 2021

Is the valigin problem fixed? I got the same issue when adding placeholder.

@eddy-lau
Copy link

eddy-lau commented Jan 5, 2021

Is the valigin problem fixed? I got the same issue when adding placeholder.

I confirmed that the latest 3.4.0 still has this issue.

@gitbrent
Copy link
Owner

Thanks for reporting this @bestis

Fixed in newest build, targeted to v3.6.0

let pptx = new PptxGenJS();
pptx.defineSlideMaster({
    title: "MASTER_SLIDE",
    background: { fill: "F1F1F1" },
    margin: [0.5, 0.25, 1.0, 0.25],
    objects: [
        {
            placeholder: {
                options: {
                    name: "header",
                    type: "title",
                    x: 0.6,
                    y: 0.2,
                    w: 6,
                    h: 1.0,
                    color: "696969",
                    fontSize: 18,
                    align: "left",
                    valign: "middle",
                    margin: 0,
                },
                text: "",
            },
        },
        {
            placeholder: {
                options: {
                    name: "title",
                    type: "title",
                    x: 0.6,
                    y: 2.2,
                    w: 6,
                    h: 1.0,
                    color: "404040",
                    fontSize: 18,
                    align: "right",
                    valign: "bottom",
                    margin: 0,
                },
                text: "METOO",
            },
        },
    ],
});

let slide = pptx.addSlide("MASTER_SLIDE");

pptx.writeFile({ fileName: "PptxGenJS-Sandbox.pptx" });

Screen Shot 2021-04-17 at 16 27 07

gitbrent added a commit that referenced this issue Apr 18, 2021
@mikemeerschaert
Copy link
Contributor

mikemeerschaert commented Apr 26, 2023

EDIT: this is kind of working, I realized I was approaching testing this differently. When adding slides to a placeholder via the library API, the valign and margin are correct, however, when viewing the master slide in powerpoint, or using a generated presentation's master slide to create a new slide manually in powerpoint, valign and margin stop working. it's a separate issue however so I'll open a new issue and submit a PR for that specific case.

Sorry but I don't think this is working anymore. I just tested on v3.12.0 and noticed that neither valign nor margin are being respected in the placeholders.

image

I think this is because of this line of code:

if (itemOpts.placeholder && target._slideLayout && target._slideLayout._slideObjects) {

You're checking if target._slideLayout exists, which will never be true, because of how the master slides are created.

The above referenced code is in the addTextDefinition method, which is called by createSlideMaster, which passes through the target that is created in the defineSlideMaster method, which defines the layout like so before passing it as the target.

const newLayout: SlideLayout = {
	_margin: props.margin || DEF_SLIDE_MARGIN_IN,
	_name: props.title,
	_presLayout: this.presLayout,
	_rels: [],
	_relsChart: [],
	_relsMedia: [],
	_slide: null,
	_slideNum: 1000 + this.slideLayouts.length + 1,
	_slideNumberProps: props.slideNumber || null,
	_slideObjects: [],
	background: props.background || null,
	bkgd: props.bkgd || null,
}

As far as I can tell, _slideLayout never gets defined, so that block of code referenced above will never return true for master slides, and the valign and margin will never be applied.

I can look into making a PR to fix this, I also noticed bullet's `characterCodes' aren't respected in placeholders either so I'll see if I can figure out how to fix that issue as well (although that's a separate issue)

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

6 participants