Improve padding logic for updatemenus by rreusser · Pull Request #989 · plotly/plotly.js

rreusser · 2016-09-28T21:25:28Z

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

rreusser · 2016-09-28T21:25:28Z

See #985
cc @etpinard

This PR improves padding logic for updatemenus. It implements pad: {t, r, b, l} at the individual-updatemenu level. I didn't apply unit testing, but did beef up the image mock to really ensure that I've tested the permutations.

The general logic:

After computing the minimum required size of the container in order to fit the content, it adds the padding onto each side.
After expanding the padding, the anchor logic is applied.

The order of operations makes a slight difference, but the general concept is that this applies within the container first, then to the overall layout as expected. I think this is by far the most intuitive and easy way to accomplish this, and meets the need.

Unfortunately it's not that straightforward to document image mocks, and I'd rather not have (literally) sixty mocks to verify all aspects of the positioning, and I think it's best to avoid testing visual alignment in the unit tests.

So on that note, here's the improved mock with the anchor annotated in blue and padding annotated in red:

rreusser · 2016-09-28T21:40:05Z

Note that I did not set a minimum value: 0 for padding. Since we don't share the full generality of the html box model, I feel like negative padding could be a usefully obscure option to have available since we don't have pixel margins to worth with.

rreusser · 2016-09-28T22:05:19Z

Relayout test to be added; battery is at 5% so will be up, but not right at the moment. Otherwise complete and reviewable.

etpinard

@rreusser this is looking great. Nice addition!

Ping me again once that relayout test is in.

etpinard · 2016-09-28T22:27:09Z

 var fontAttrs = require('../../plots/font_attributes');
 var colorAttrs = require('../color/attributes');
 var extendFlat = require('../../lib/extend').extendFlat;
+var padAttrs = require('../../plots/pad_attributes');


That's where pad_attributes.js belongs at the moment, but I hate to see so many files pile up in the src/plots/ root.
Maybe we could add a src/plots/common/folder? or src/components/common/? or even a src/common/ for similar shared modules.

Agreed. Font attributes would probably go with it? At least internal reorg is easy refactoring (especially since static requires so that errors fail bundling), so can try to cut down on plots/ bit by bit (pun?)

Font attributes would probably go with it?
yep, exactly.
so can try to cut down on plots/ bit by bit (pun?)
No action required in this PR. I just wanted to mention something I've been thinking about for a while now.

etpinard · 2016-09-28T22:36:51Z

+ l: paddedWidth * ({right: 1, center: 0.5}[xanchor] || 0),
+ r: paddedWidth * ({left: 1, center: 0.5}[xanchor] || 0),
+ b: paddedHeight * ({top: 1, middle: 0.5}[yanchor] || 0),
+ t: paddedHeight * ({bottom: 1, middle: 0.5}[yanchor] || 0)


Wow. That was easy. I love it!

rreusser · 2016-09-28T22:39:09Z

@etpinard will do. Have run out of steam at the moment, but will aim to have it good to go first thing tomorrow.

rreusser · 2016-09-29T13:00:28Z

ping @etpinard — added relayout tests that at least confirms a couple basic things about padding getting updated and making some sense.

etpinard · 2016-09-29T15:23:26Z

This is working great. Thanks! 💃

@rreusser merge away

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Improve padding logic for updatemenus
05ac586

rreusser added the status: reviewable label Sep 28, 2016

etpinard added the feature something new label Sep 28, 2016

etpinard added this to the v1.18.0 milestone Sep 28, 2016

etpinard suggested changes Sep 28, 2016
View reviewed changes

Add jasmine tests for updatemenus padding
4604341

etpinard approved these changes Sep 29, 2016
View reviewed changes

etpinard mentioned this pull request Sep 29, 2016
Unify component pad logic #997
Closed

rreusser merged commit fce36fa into masterSep 29, 2016

rreusser deleted the fix-updatemenus-padding branch September 29, 2016 16:24

Uh oh!

Uh oh!

Conversation

rreusser commented Sep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rreusser commented Sep 28, 2016

Uh oh!

rreusser commented Sep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etpinard left a comment

Choose a reason for hiding this comment

Uh oh!

etpinardSep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rreusserSep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etpinardSep 29, 2016

Choose a reason for hiding this comment

Uh oh!

etpinardSep 28, 2016

Choose a reason for hiding this comment

Uh oh!

rreusser commented Sep 28, 2016

Uh oh!

rreusser commented Sep 29, 2016

Uh oh!

etpinard commented Sep 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

rreusser commented Sep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rreusser commented Sep 28, 2016

Uh oh!

rreusser commented Sep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

etpinard left a comment

Choose a reason for hiding this comment

Uh oh!

etpinardSep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rreusserSep 28, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etpinardSep 29, 2016

Choose a reason for hiding this comment

Uh oh!

etpinardSep 28, 2016

Choose a reason for hiding this comment

Uh oh!

rreusser commented Sep 28, 2016

Uh oh!

rreusser commented Sep 29, 2016

Uh oh!

etpinard commented Sep 29, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

rreusser commented Sep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading

etpinardSep 28, 2016•
edited
Loading

rreusserSep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading

etpinardSep 28, 2016•
edited
Loading

rreusserSep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading

etpinardSep 28, 2016•
edited
Loading

rreusserSep 28, 2016•
edited
Loading

rreusser commented Sep 28, 2016•
edited
Loading