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

Provide a blank default global layout option #4319

Merged
merged 1 commit into from Jun 4, 2017
Merged

Conversation

etimberg
Copy link
Member

@etimberg etimberg commented Jun 4, 2017

Resolves #4309

@etimberg etimberg added this to the Version 2.7 milestone Jun 4, 2017
src/core/core.js Outdated
@@ -32,6 +32,11 @@ module.exports = function() {
// Element defaults defined in element extensions
elements: {},

// Layout options such as padding
layout: {
padding: 0
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to expand the default option to the object form to get a better options merge?

// defaults:
layout: {
    padding: {
        top: 0,
        right: 0,
        bottom: 0,
        left: 0
    }
}

// merged with:
layout: {
    padding: {
        top: 16
    }
}

// produces:
layout: {
    padding: {
        top: 16,
        right: 0,
        bottom: 0,
        left: 0
    }
}

// instead of:
layout: {
    padding: {
        top: 16
        //right: undefined
        //bottom: undefined
        //left: undefined
    }
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, that's a good idea. Will update

@simonbrunel simonbrunel merged commit fd49ac9 into master Jun 4, 2017
@simonbrunel simonbrunel deleted the fix/4309 branch June 4, 2017 17:32
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants