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

Remove defaultProps from Page component (enables auto-incrementing page name) #296

Merged
merged 1 commit into from
Apr 22, 2018

Conversation

jaridmargolin
Copy link
Collaborator

Problem

The default name prop on the Page component provides no value. It in-fact has negative consequences. If we create multiple pages with no specified name, they will all default to Page 1. If instead we let the name remain undefined, Sketch will create the page using an auto-incremented name (Page 1, Page 2, Page 3, etc...)

Solution

Remove the defaultProps.name definition :)

Implementation Notes

This is a larger concern than just this particular PR, but I don't see where or how I can create tests to confirm functionality that involves Sketch's behavior (in this case, the behavior of document.addBlankPage). I was able to manually test this by opening sketch and rendering multiple Page components without specifying the name.

@mathieudutour
Copy link
Collaborator

where or how I can create tests to confirm functionality that involves Sketch's behavior

I don't think there is a way to do it now but if you want to look at it, have a look at https://github.com/skpm/skpm/tree/master/packages/test-runner

@mathieudutour mathieudutour merged commit e0fed5b into airbnb:master Apr 22, 2018
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.

2 participants