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 padding(_) #23

Closed
wants to merge 1 commit into from
Closed

Fix padding(_) #23

wants to merge 1 commit into from

Conversation

Legion4444
Copy link
Contributor

@Legion4444 Legion4444 commented Jul 14, 2019

As it currently stands, my. padding(_) returns an error of "Cannot read property 'paddingY' of undefined".

my.padding() does the chain "my.paddingX().paddingY()"; however, "paddingX()" does not actually return anything like Left/Right/Top/Bottom does, and "paddingY(_)" cannot be called on undefined.

My proposed solution is to separate "my.paddingX().paddingY()" into two separate calls "my.paddingX()" and "my.paddingY()".

As it currently stands, my. padding(_) returns an error of "Cannot read property 'paddingY' of undefined". 

We doing the chain "my.paddingX(_).paddingY(_)"; however, "paddingX(_)" does not actually return anything like Left/Right/Top/Bottom does, and "paddingY(_)" cannot be called on undefined. 

My proposed solution is to separate "my.paddingX(_).paddingY(_)" into two separate calls "my.paddingX(_)" and "my.paddingY(_)".
Copy link
Owner

@curran curran left a comment

Choose a reason for hiding this comment

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

I understand the problem you've described. However, I think a better solution would be to have everything return my, instead of this change you suggest, which feels more like a workaround (treating the symptom, not the disease).

I suggest you modify this PR such that the following methods return my:

  my.paddingX = function(_) {
    my.paddingLeft(_).paddingRight(_);
  };

  my.paddingY = function(_) {
    my.paddingTop(_).paddingBottom(_);
  };

  my.padding = function(_) {
    my.paddingX(_).paddingY(_);
  };

Like this:

  my.paddingX = function(_) {
    return my.paddingLeft(_).paddingRight(_);
  };

  my.paddingY = function(_) {
    return my.paddingTop(_).paddingBottom(_);
  };

  my.padding = function(_) {
    return my.paddingX(_).paddingY(_);
  };

I think that should work..

Thank you for your effort here!

@@ -242,7 +242,8 @@ function areaLabel(area) {
};

my.padding = function(_) {
my.paddingX(_).paddingY(_);
my.paddingX(_)
Copy link
Owner

Choose a reason for hiding this comment

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

Missing a semicolon here.

@Legion4444 Legion4444 closed this Jul 15, 2019
@Legion4444 Legion4444 deleted the patch-1 branch July 15, 2019 12:47
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