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

Fix lineWidth object bug with fillColor: null #1002

Merged
merged 1 commit into from
Oct 10, 2023

Conversation

mmghv
Copy link
Collaborator

@mmghv mmghv commented Oct 10, 2023

Fixes #1001

Refs : #730, #961, #978

fillColor: null caused cells to have full borders even when lineWidth is an object with partial borders.

This is caused by calling rect() to draw the background with null as fillStyle when fillColor is null :

const fillStyle = getFillStyle(0, fillColor)
doc.rect(cell.x, cursor.y, cell.width, cell.height, fillStyle)

Reproduce code

doc.autoTable({
  head: [['Name', 'Email', 'Country']],
  body: [
    ['David', 'david@example.com', 'Sweden'],
    ['Castille', 'castille@example.com', 'Spain'],
  ],
  theme: 'plain',
  styles: {
    // fillColor: null,
    lineColor: 0,
    lineWidth: {top: 0, bottom: 0.3, right: 0, left: 0},
  },
})

Before

image

After

image

Comment on lines -116 to +119
rect(x: number, y: number, width: number, height: number, fillStyle: 'S' | 'F' | 'DF' | 'FD' | null) {
if (!['S', 'F', 'DF', 'FD', null].some((v) => v === fillStyle)) {
throw new TypeError(
`Invalid value '${fillStyle}' passed to rect. Allowed values are: 'S', 'F', 'DF', 'FD', null`
)
}
rect(x: number, y: number, width: number, height: number, fillStyle: 'S' | 'F' | 'DF' | 'FD') {
// null is excluded from fillStyle possible values because it isn't needed
// and is prone to bugs as it's used to postpone setting the style
// https://rawgit.com/MrRio/jsPDF/master/docs/jsPDF.html#rect
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've excluded null from being used as fillStyle with rect() which is intended to postpone setting the style when drawing multiple shapes at once, this was first introduced in this PR. this shouldn't be used as it could lead to bugs like this when misused.

@simonbengtsson
Copy link
Owner

simonbengtsson commented Oct 10, 2023

Awesome! Tagging @MikeDabrowski in case he wants to review as well.

@simonbengtsson simonbengtsson removed their assignment Oct 10, 2023
@mmghv mmghv deleted the bug/lineWidth-object-fillColor branch October 10, 2023 12:25
@MikeDabrowski
Copy link
Contributor

Yep, brilliant! Thx for the fix and tagging.
I tested it on my project and had no regressions

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.

lineWidth object doesn't work correctly with plain theme or fillColor: null
3 participants