Style attribute out of sync when changing between a css shorthand and specific property like border to border-left

[brantwedel]

I'm submitting a bug report

• Library Version: 1.6.0

Please tell us about your environment:

• Operating System: OSX 10.13.4

• Node Version: 8.9.1

• NPM Version: 4.6.1

• JSPM OR Webpack AND Version JSPM 0.16.53

• Browser: Chrome 66.0.3359.139 | FF 59.0.2 | all?

• Language: all

Current behavior: When toggling between a css shorthand property and a specific property like border and border-left in a css/style attribute, the styles get out of sync / don't get applied.

Example gistrun: https://gist.run/?id=b6dac1c04a230db45afc8825f499edf5

Problem code: https://github.com/aurelia/binding/blob/master/src/element-observation.js#L124 style.removeProperty('border') will also remove related properties like 'border-left' also, style.removeProperty('border-width') will also remove 'border-left-width', etc.

Simplest Example:

  <div click.delegate="toggle = !toggle" css="${toggle ? 'border: 2px solid green' : 'border-left: 2px solid green'}">
    Click: border will be removed completely, instead of leaving a left border
  </div>

Expected/desired behavior:

Actual element styles should always stay in sync with aurelia css/style bindings (while maintaining compatability with other frameworks that mutate styles)

• What is the motivation / use case for changing the behavior?

When this happens, it is incredibly mysterious at first, and hard to track down :D

[stsje]

Good find :)! 👍

[Alexander-Taran]

could we refactor the method to calculate "would be removed properties" and remove them before applying the new ones?

[brantwedel]

Sadly, it won't be that easy, probably going to have to resort to parsing/setting the style attribute text, as .style.setProperty( also tries to be "smart".

ex: this simple binding will cause havoc with .style.setProperty( (Edit: actually this case will 'happen' to work, because removing 'border' would actually remove the additional properties ... but you can see what is happening):

<div css="${toggle ? 'border: 4px solid black; border-left: 2px solid black' : '' }"></div>

div = document.createElement('div')
// div => <div>​</div>​

div.style.setProperty('border', '4px solid black');
// div => <div style=​"border:​ 4px solid black;​">​</div>​

div.style.setProperty('border-left', '2px solid black');
// will coerce/optimize every other property of border (I think to make style order independent)
// div => <div style=​"border-width:​ 4px 4px 4px 2px;​ border-style:​ solid;​ border-color:​ black;​ border-image:​ initial;​">​</div>​

Most likely have to resort to manipulating with setAttribute('style') getAttribute('style'), because it wont try to outsmart your styles:

div = document.createElement('div')
// div => <div>​</div>​

function addStyle(styleAttrText, style) {
    // parse style text, and add/replace (and re-order) as necessary
    return styleAttrText ? styleAttrText + (!styleAttrText.endsWith(';') ? '; ' : '') + style : style;
}
function removeStyle(styleAttrText, style) {
    // parse style text, and remove as necessary
    return styleAttrText.replace(style, '');
}

div.setAttribute('style', addStyle(div.getAttribute('style'), 'border: 4px solid black;'));
// div => <div style=​"border:​ 4px solid black;​">​</div>​

div.setAttribute('style', addStyle(div.getAttribute('style'), 'border-left: 2px solid black;'));
// div => <div style=​"border:​ 4px solid black;​border-left:​ 2px solid black;​">​</div>​

[Alexander-Taran]

In your case you want too much magic. And there is an easy workaround for your case. I was thinking just to make current logic more stable.. because removing the shortcut property first and then applying explicit border-left - will be a correct solution. having inline styles is just.. hacky.. why not have 2 classes? The fact that you can do it - does not mean that it is a good way to do it. Actually your example makes me think that css binding should be removed once and for all.. (-:

[brantwedel]

In my actual use case, I have resizable dock panels and I need to know the border sizes to ensure docking/pinning/resizing behavior accounts for them (border gets thicker when pinning). So setting it all in my js component, instead of having as much coupling with the css seemed good, since other properties like width, were already completely controlled by js/dragging.

Once I figured out the issue, the workaround was easy ... so maybe some documentation warning about mixing shorthand and specific css properties would be sufficient :D

My actual use-case / error was basically this (cleaner and in well named computedFrom getters of course, also I do have classes for .is-*, but the js needs to know the border widths): <dock-panel css="${ isShowing ? 'border-left-width: ' + (isPinned ? borderPinnedWidth : borderWidth) + 'px' : 'border-width: 0px' }; width: ${panelWidth}px">

Workaround, is to not be lazy and always specify border-left-width: <dock-panel css="${ isShowing ? 'border-left-width: ' + (isPinned ? borderPinnedWidth : borderWidth) + 'px' : 'border-left-width: 0px' }; width: ${panelWidth}px">

Edit: Refactoring the order of removeProperty would fix this specific case. As a user that didn't previously know how setProperty and removeProperty behaved (or that that is what Aurelia used), I didn't want "magic", I wanted the style attribute text to be set to value of the css interpolated text :D

[Alexander-Taran]

@brantwedel if you don't use border radius, you might want to use outline or box-shadow.. for visual effects.. they don't affect the layout.. and positioning. so you could rely on constant border-width. have a look: https://codepen.io/Alexander-Taran/pen/zjdRgo

[brantwedel]

Thanmks for the suggestions, shadow as border is is what I had earlier but it would cover the other dock panels docked against since they are outside the box-model which encroaches into the panel to the side, I have tried all the things and am aware of the csss.

I was going to try shadow + margin ... but the dock uses flexbox and I didnt want to figure out the right set of proprties to not collapse margins in all browsers ... I'm sure it's possible, but the style attr for border works very well :D Edit: at a glance shadow/outline + margin does seem to work decently based on that pen https://codepen.io/anon/pen/erEPpo ... if I were to change my current code, I would probably try box-sizing: content-box; ... in component a panel can be set to overlay content as well, so it's nested more ... both would need the same border and/or box model ... it's complicated :-p

@Alexander-Taran I'll send you a link to my dock-panel git repo if I make it public and you can review more 🤣

In any case, with knowlede of this issue, it's quite easy to avoid ... but seemed magically broken initially :D

[Alexander-Taran]

@brantwedel let's close this (-: