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

!important styles don't get overriden #44

Closed
mderazon opened this issue Mar 4, 2015 · 9 comments · Fixed by #45
Closed

!important styles don't get overriden #44

mderazon opened this issue Mar 4, 2015 · 9 comments · Fixed by #45
Labels

Comments

@mderazon
Copy link
Contributor

mderazon commented Mar 4, 2015

I have a React.js component that is setting the height of the component as an inline style and I have no control over it.
To make it work with buggyfill, I had to create a class with !important to override the precedency of the inline styles of the lib. Therefor I have something like this in my CSS file:

.map {
  height: 85vh !important;
}

When debugging on IOS7, I see the fixed px class that is the result of buggyfill, but the above class definition still takes precedency over the 3 (the buggyfill version, the inline version and the !important class version)

Any ideas how to bypass this ?

@mderazon
Copy link
Contributor Author

mderazon commented Mar 4, 2015

found a workaround.
changed line 326
from

return (_number * _base) + 'px';

to

return (_number * _base) + 'px !important';

@rodneyrehm rodneyrehm added the bug label Mar 5, 2015
@rodneyrehm
Copy link
Owner

You could try a bit more generic approach to solving the issue in replaceValues():

function replaceValues(match, number, unit, source) {
  var _base = dimensions[unit];
  var _number = parseFloat(number) / 100;
  var _important = source.indexOf('!important') !== -1 ? ' !important' : '';
  return (_number * _base) + 'px' + _important;
}

let me know if that works for you (I haven't tested it) so we may improve the buggyfill instead of dealing with custom workarounds…

@mderazon
Copy link
Contributor Author

mderazon commented Mar 5, 2015

You've changed replaceValues signature from match, number, unit to match, number, unit, source without changing the caller function ?
Am I missing something ?

It doesn't work, tells me that variable source is undefined

@rodneyrehm
Copy link
Owner

The caller is String.prototype.replace() and it knows the signature of the supplied callback. I was missing the offset parameter, though:

function replaceValues(match, number, unit, offset, source) {
  var _base = dimensions[unit];
  var _number = parseFloat(number) / 100;
  var _important = source.indexOf('!important') !== -1 ? ' !important' : '';
  return (_number * _base) + 'px' + _important;
}

@mderazon
Copy link
Contributor Author

mderazon commented Mar 5, 2015

that did fix the "source undefined" issue but for some reason !important is not being passed to the source.

For example, in my stylesheet I define

.map {
    height: 75vh !important;
}

and printing source variable in replaceValues() I see

75vh

instead of

75vh !important

@rodneyrehm
Copy link
Owner

Then we should investigate how to get that !important accessed…

@mderazon
Copy link
Contributor Author

mderazon commented Mar 5, 2015

On line 232 you do

forEach.call(rule.style, function(name) {
  var value = rule.style.getPropertyValue(name);
  viewportUnitExpression.lastIndex = 0;
  if (viewportUnitExpression.test(value)) {
    declarations.push([rule, name, value]);
    options.hacks && options.hacks.findDeclarations(declarations, rule, name, value);
  }
});

CSSStyleDeclaration.getPropertyValue() only gets the property value,

We can use it with CSSStyleDeclaration.getPropertyPriority() (see here)

So it will be something like

forEach.call(rule.style, function(name) {
  var priority = '';
  if (rule.style.getPropertyPriority(name)) {
    priority = ' !important';
  }
  var value = rule.style.getPropertyValue(name) + priority;
  viewportUnitExpression.lastIndex = 0;
  if (viewportUnitExpression.test(value)) {
    declarations.push([rule, name, value]);
    options.hacks && options.hacks.findDeclarations(declarations, rule, name, value);
  }
});

What do you think ?

@mderazon
Copy link
Contributor Author

mderazon commented Mar 5, 2015

tested it with my use case, and it looks like it's fine.

Tried using both

.map {
  height: 85vh !important;
}

and

.map {
  height: 85vh;
}

first one converted to 360px !important and the second one to 360px on my iphone 4 so it looks like it's working.

Also, it's safe to assume that getPropertyPriority() returns either "important" or empty string ""

Are there any other implications you're aware of ?

@rodneyrehm
Copy link
Owner

Good Work!

I think pushing the boolean returned by rule.style.getPropertyPriority(name) onto the declarations list declarations.push([rule, name, value, priority]); is the cleaner (as in more obvious) solution, but one that requires a lot more code to be changed, whereas your propsal is limited to a single function. So I'd go with your proposal, rewritten to a 4-line change:

forEach.call(rule.style, function(name) {
  var value = rule.style.getPropertyValue(name);
  // preserve those !important rules
  if (rule.style.getPropertyPriority(name)) {
    value += ' !important';
  }

  viewportUnitExpression.lastIndex = 0;
  if (viewportUnitExpression.test(value)) {
    declarations.push([rule, name, value]);
    options.hacks && options.hacks.findDeclarations(declarations, rule, name, value);
  }
});

Do you want to send a PR for this? (If so, please also add and example to the test.css, which sort of functions as a manual test)

mderazon added a commit to mderazon/viewport-units-buggyfill that referenced this issue Mar 8, 2015
rodneyrehm added a commit that referenced this issue Mar 10, 2015
fixing the preservation of !important rules - #44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants