-
Notifications
You must be signed in to change notification settings - Fork 1
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
Implement ChordDiagram #826
base: master
Are you sure you want to change the base?
Conversation
Somehow we're going to need the ability to get the width and height from what is rendered so that we can properly increment Think it might make more sense to have a width and height property on the JsPDFRenderer so instead of const renderer = new JsPDFRenderer(this.doc, { x: 50, y: 50, scale: 0.1 }); it could be const renderer = new JsPDFRenderer(this.doc, { x: 50, y: 50, width: 40 }); This could just internally just calculate the "scale" against whatever the defined "width" was inside the ChordDiagram class. Related to properly incrementing What if the general idea was something along the lines of this: Note how in this example I even removed the setting of x and y from the JsPDFRenderer and moved that to props in the diagram.render() so we could share some config elements but render multiple beside eachother renderChordDiagrams(): void {
const renderer = new JsPDFRenderer(this.doc, { width: 35 });
// Assuming this would be getting ChordDiagrams from the song
const chordDiagrams = [
new ChordDiagram({
...
}),
new ChordDiagram({
...
}),
];
chordDiagrams.forEach((diagram) => {
const { width, height } = diagram.measure(); // or getDimensions()
if (this.x + width > this.columnWidth) {
this.y += height + 10 // diagram y spacing configurable
this.carriageReturn();
}
this.x += width + 10 // diagram x spacing configurable;
if (this.y + height > this.doc.internal.pageSize.getHeight()) {
// Move to the next column if the diagram doesn't fit
// problem with not creating a new "render" instance is that the this.doc won't be updated to hold new page if it moves to next page
// render functon should either get a new instance, or allow this.doc to be passed into the render function
this.moveToNextColumn();
}
diagram.render(renderer,{ x: this.x, y: this.y}, this.doc);
});
}
} ^ actual code would be different because I know this would have a couple bugs with extra spacing and not left aligning the first diagram but mostly I'm just trying to identify how I know we'll need to use it. thoughts? |
@isaiahdahl I changed the rendering a bit. I removed all spacing around the diagram, and you can now set the desired width instead of the scale. The renderer exposes the |
2fb03d9
to
79f4486
Compare
79f4486
to
13d6f66
Compare
13d6f66
to
3ea7c3e
Compare
3ea7c3e
to
37e5163
Compare
@isaiahdahl I implemented basic rendering; chord diagrams are rendered at the end, and it checks the available space. If we want to work out more possibilities/configurations it might be good to have a small brainstorm to work it out. Agree to merge this and improve after? |
Will have a look, but I'm onboard with that! I'm pretty flexible so just let me know some times that work for you. |
Resolves #820