SketchAPI icon indicating copy to clipboard operation
SketchAPI copied to clipboard

Creating a rectangle via the API uses the default border style in some cases

Open KevinGutowski opened this issue 5 years ago • 7 comments

Prerequisite setup

  • Make sure your default layer style has something with a border (try a red border for greater clarity). To set a default layer style go to Layer- > Style -> Set as Default Style

Details

Currently if you create a rectangle with no styles it won’t have any fills or borders.

let sketch = require('sketch')
let document = sketch.getSelectedDocument()
let page = document.selectedPage
page.layers = []
let Artboard = sketch.Artboard
let myArtboard = new Artboard({
    parent: page,
    frame: { x: 0, y: 0, width: 400, height: 400 }
})
let ShapePath = sketch.ShapePath
let mySquare = new ShapePath({
    parent: myArtboard, 
    frame: { x: 53, y: 213, width: 122, height: 122 }
})

However, if you create a rectangle with a fill, it will inherit the default border property.

let mySquare = new ShapePath({
    parent: myArtboard, 
    frame: { x: 53, y: 213, width: 122, height: 122 },
    style: { fills: ['#35E6C9']}
})

If you are someone who changed the default style properties to not include a border then its very easy to look over this fact when developing a Sketch plugin.

Interestingly, if you do the reverse (add a border but no fill) the default fill does not get applied.

KevinGutowski avatar Apr 27 '19 20:04 KevinGutowski

Moreover, this makes me a little worried that there are other preferences that I have made over the years of using Sketch that have adverse effects in my plugin development.

KevinGutowski avatar Apr 27 '19 20:04 KevinGutowski

That's a good point (and there is clearly an inconsistency that should be fixed) but ultimately, I don't know what the solution is. On one hand I'm all for reproducibility and I'd agree that the created style should be as plain as possible. One the other hand, the default of a style is clearly defined by the user, and so I'd be tempted to define that as the API default. If you want your style not to respect it, then you override it.

The way I see it, there is 2 ways to go:

  • new Shape() uses the default style set by the user.
  • new Shape() uses a blank style and we expose Style.fromUserDefault() to create a style from the user default.

Something to keep in mind is that there is no blank text style so we would need to maintain one if we want to go that way

mathieudutour avatar Apr 29 '19 12:04 mathieudutour

I feel that the latter is the better way to go. If you are pragmatically drawing shapes, it's very difficult to consider the fact that the user’s settings could be changing what you expect for all users.

Something to keep in mind is that there is no blank text style so we would need to maintain one if we want to go that way

I don’t understand this. Well, I get that there is no blank text style but not sure what you mean by "we would need to maintain one"

KevinGutowski avatar Apr 29 '19 15:04 KevinGutowski

We would need to define what a blank text style means (eg. what font, size, etc.) and every time properties change, we need to update it.

mathieudutour avatar Apr 29 '19 16:04 mathieudutour

Hmmm I guess this is the behavior that I am expecting:

let mySquare = new ShapePath({
    parent: myArtboard, 
    frame: { x: 53, y: 213, width: 122, height: 122 }
})
//no fill or border set

let mySquare = new ShapePath({
    parent: myArtboard, 
    frame: { x: 53, y: 213, width: 122, height: 122 },
    style: { fills: ['#35E6C9']}
})
//only fill set

let mySquare = new ShapePath({
    parent: myArtboard, 
    frame: { x: 53, y: 213, width: 122, height: 122 },
    style: { borders: ['#35E6C9']}
})
//only 1px border set

And for text

let text = new Text({
  text: 'my text',
  parent: myArtboard
})
//12px, Helvetica, left alignment

Text would have a default unlike ShapePath

KevinGutowski avatar Apr 30 '19 06:04 KevinGutowski

I'm tempted to sit on this issue for a bit. That would be a breaking change and there are some important changes coming styles so let's reduce the churn to a minimum.

mathieudutour avatar May 03 '19 13:05 mathieudutour

Yeah, that seems fine. I haven't thought of a decent solution to this either :/

KevinGutowski avatar May 03 '19 18:05 KevinGutowski