Suggestion to improve selector flexibility

[guitarino]

Hello there.

Thanks for the excellent library.

I really like the library and I don't demand on any changes, but I thought I'd share a way I use to improve the way CSS selectors are treated.

Firstly, I encountered a problem with DOMClass when I tried to select the following selector: 'my-element[attr]'. Since restyle adds a mandatory space between 'my-element' and '[attr]' when putting '[attr]' as a property key, it makes it impossible to select my-element that has a particular attribute.

Secondly, it is not too nice when you have more than 1 selector for the same CSS properties. For example, in the following case, we are forced to write all selectors on one line:

restyle({
  '.toolbar-start-container, .toolbar-center-container, .toolbar-end-container, .toolbar-total-container': {
    height: 50
  }
})

A good solution is for restyle to actually accept an array instead of an object, like follows:

restyle([
  '.toolbar-start-container',
  '.toolbar-center-container',
  '.toolbar-end-container',
  '.toolbar-total-container', {
    height: 50
  }
])

And then, reformat the array to fit the intended object.

So, in order to solve both of those issues I'm using the following wrapper for the restyle:

var AddCSS = function() {
  var prefix = '';
  var css;
  var restyle_css = {};

  if(arguments.length > 1) {
    prefix = arguments[0];
    css = arguments[1];
  }
  else if(arguments.length == 1) {
    css = arguments[0];
  }
  else {
    throw new Error('not enough arguments');
  }

  var selector = '';
  for(var i = 0; i < css.length; i++) {
    var cssLine = css[i];
    if(typeof cssLine === "string") {
      if(selector.length) selector += ', ';
      selector += (prefix + cssLine);
    }
    else if(typeof cssLine === "object") {
      restyle_css[selector] = cssLine;
      selector = '';
    }
  }

  return restyle(restyle_css);
};

The first problem is solved by using an idea of 'prefix' instead of 'component', i.e. we just add any prefix to each selector, and it is up to the user to decide whether that prefix will include the space. And the second problem is solved by using the array and reformatting

I think it would be beneficial if the support for this was added to the library if the author decides that it's a good idea. Cheers.

[WebReflection]

Firstly, I encountered a problem with DOMClass when I tried to select the following selector: 'my-element[attr]'. Since restyle adds a mandatory space between 'my-element' and '[attr]' when putting '[attr]' as a property key, it makes it impossible to select my-element that has a particular attribute.

the component bit is used to encapsulate, simulating a template, the component itself and its nodes.

my-component [rel=nofollow] that suddenly would become my-component[rel=nofollow] would break badly but I understand your issue.

Would duplicating the selector to check both the component itself and its nodes be an option? Something like my-component[rel=nofollow], my-component [rel=nofollow]

Since it's in any case not extremely elegant, do you have any better suggestion?

Secondly, it is not too nice when you have more than 1 selector for the same CSS properties. For example, in the following case, we are forced to write all selectors on one line:

Well, ES2015 supports runtime attributes.

var o = {[[
  '.toolbar-start-container',
  '.toolbar-center-container',
  '.toolbar-end-container',
  '.toolbar-total-container'
]]: {
    height: 50
}};

You can easily use a transpiler that targets this feature only so this is a won't fix due future, present for most modern browsers, and backward compatible, if using transpilers.

So, we are left to a good solution for the first case. Has any other library author solved this already? (absurd or other similar libraries)

Cheers

[WebReflection]

edited actually, since Array#join by default uses the string ',', you don't even need to explicitly do that

[guitarino]

Thanks for a quick response.

With regards to the first issue, I think it might make more sense to add another property, call it cssGlobal (or sth else) that would not be prefixed.

DOMClass({
  name: 'my-element',

  cssGlobal: {
    'my-element': {
      height: 50
    },
    'my-element[attr]': {
      marginRight: 10
    },
    'my-element[attr] my-element-child': {
      marginLeft: 10
    }
  },

  css: {
    'my-element-child': {
      height: 15
    }
  }
});

This will not break backward compatibility and also solve the issue.

With regards to the second issue, I still think it's worth looking into, for a few reasons: 1) If you use runtime attributes, then the problem arises that, when used in a DOMClass component, they will not be automatically prefixed with the component tag. For example, if I do the following:

DOMClass({
  name: 'my-element',

  css: {[[
    '.toolbar-start-container',
    '.toolbar-center-container',
    '.toolbar-end-container',
    '.toolbar-total-container'
  ]]: {
      height: 50
  }}
});

Will result in css selector

my-element .toolbar-start-container,
.toolbar-center-container,
.toolbar-end-container,
.toolbar-total-container

But we expect it to be

my-element .toolbar-start-container,
my-element .toolbar-center-container,
my-element .toolbar-end-container,
my-element .toolbar-total-container

2) It actually feels more natural to do the following:

DOMClass({
  name: 'my-element',

  css: [
    '.toolbar-start-container',
    '.toolbar-center-container',
    '.toolbar-end-container',
    '.toolbar-total-container', {
      height: 50
    },
    '.toolbar-start-container:before',
    '.toolbar-center-container:before',
    '.toolbar-end-container:before',
    '.toolbar-total-container:before', {
      content: '" "',
      verticalAlign: 'middle'
    },
    '.toolbar-start',
    '.toolbar-center',
    '.toolbar-end',
    '.toolbar-total > span', {
      verticalAlign: 'middle'
    }
  ]
});

This actually feels like you're writing real CSS, without the need to put '[[' and ']]'

Compare that to using runtime attributes:

DOMClass({
  name: 'my-element',

  css: {
    [['.toolbar-start-container',
      'my-element .toolbar-center-container',
      'my-element .toolbar-end-container',
      'my-element .toolbar-total-container']]: {
        height: 50
    },
    [['.toolbar-start-container:before',
      'my-element .toolbar-center-container:before',
      'my-element .toolbar-end-container:before',
      'my-element .toolbar-total-container:before']]: {
        content: '" "',
        verticalAlign: 'middle'
    },
    [['.toolbar-start',
      'my-element .toolbar-center',
      'my-element .toolbar-end',
      'my-element .toolbar-total > span']]: {
        verticalAlign: 'middle'
    }
  }
});

Also, a minor issue is that there is a slight overhead with converting arrays to strings (probably tiny, but still exists).

[WebReflection]

Problem is, Arrays have already a meaning so i'm not sure . Will think about it

On Friday, 28 October 2016, guitarino notifications@github.com wrote:

Thanks for a quick response.

With regards to the first issue, I think it might make more sense to add another property, call it cssGlobal (or sth else) that would not be prefixed.

DOMClass({ name: 'my-element',

cssGlobal: { 'my-element': { height: 50 }, 'my-element[attr]': { marginRight: 10 }, 'my-element[attr] my-element-child': { marginLeft: 10 } },

css: { 'my-element-child': { height: 15 } } });

This will not break backward compatibility and also solve the issue.

With regards to the second issue, I still think it's worth looking into, for a few reasons: 1) If you use runtime attributes, then the problem arises that, when used in a DOMClass component, they will not be automatically prefixed with the component tag. For example, if I do the following:

DOMClass({ name: 'my-element',

css: {[[ '.toolbar-start-container', '.toolbar-center-container', '.toolbar-end-container', '.toolbar-total-container' ]]: { height: 50 }} });

Will result in css selector

my-element .toolbar-start-container, .toolbar-center-container, .toolbar-end-container, .toolbar-total-container

But we expect it to be

my-element .toolbar-start-container, my-element .toolbar-center-container, my-element .toolbar-end-container, my-element .toolbar-total-container

2) It actually feels more natural to do the following:

DOMClass({ name: 'my-element',

css: [ '.toolbar-start-container', '.toolbar-center-container', '.toolbar-end-container', '.toolbar-total-container', { height: 50 }, '.toolbar-start-container:before', '.toolbar-center-container:before', '.toolbar-end-container:before', '.toolbar-total-container:before', { content: '" "', verticalAlign: 'middle' }, '.toolbar-start', '.toolbar-center', '.toolbar-end', '.toolbar-total > span', { verticalAlign: 'middle' } ] });

This actually feels like you're writing real CSS, without the need to put '[[' and ']]'

Compare that to using runtime attributes:

DOMClass({ name: 'my-element',

css: { [['.toolbar-start-container', 'my-element .toolbar-center-container', 'my-element .toolbar-end-container', 'my-element .toolbar-total-container']]: { height: 50 }, [['.toolbar-start-container:before', 'my-element .toolbar-center-container:before', 'my-element .toolbar-end-container:before', 'my-element .toolbar-total-container:before']]: { content: '" "', verticalAlign: 'middle' }, [['.toolbar-start', 'my-element .toolbar-center', 'my-element .toolbar-end', 'my-element .toolbar-total > span']]: { verticalAlign: 'middle' } } });

Also, a minor issue is that there is a slight overhead with converting arrays to strings (probably tiny, but still exists).

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/WebReflection/restyle/issues/4#issuecomment-256995352, or mute the thread https://github.com/notifications/unsubscribe-auth/AAFO9QNPtQfTVfDRwhg3E4BdO-qkkQaSks5q4kAwgaJpZM4Kjfsh .

[WebReflection]

do you mind trying out this version and tell me if it does what you are looking for? https://github.com/WebReflection/restyle/blob/master/build/restyle.max.js

[guitarino]

I noticed one problem: If you do something like:

restyle("component", [
    '.first-element',
    '.second-element', {
        height: 50
    }
]);

It only prepends 'component ' to the '.first-element', but not the '.second-element'. So the generated selector will be

component .first-element, .second-element

I think the expected behaviour should be:

component .first-element, component .second-element

I haven't tested deeply, but the rest seems good to me right now.

[WebReflection]

Right ... current version I've just pushed should solve that but, at the same time, I've realized restyle has never allowed comma separated / grouped definitions.

Your first example mislead me, at the same time it's something I'd probably expect.

[WebReflection]

Anyway, for now v6.0.0 should work as you expect. I might actually make this module smarter and group definitions instead of splitting all of them but right now that's less important.