Adding classes to cart td and th for easier css styling #1281

Closed
wants to merge 1 commit into
from

Projects

None yet

5 participants

Member

Styling using td[data-hook="..."] is going to break older browsers. Aside from polyfills, this is the best cross-browser solution that I'm aware of. Thoughts?

schof commented on 950c18d Mar 16, 2012

@BDQ and @devilcoders - any thoughts on these proposed changes?

@BDQ BDQ was assigned Mar 19, 2012
Member
BDQ commented Mar 20, 2012

I don't have any objections, which "older browsers" are we talking about?

Member

The browser I was thinking of was IE6, but I'm not supporting IE6 anymore in my development. Without checking, I just assumed that IE7 wouldn't have attribute selector support – but I was wrong. Sorry about that.

I think the changes on _form.html.erb would still be useful to allow easy styling + deface selection. I feel as though class selectors are a bit cleaner and more self-documenting than attribute selectors, but having both would clutter things up. I'm guessing reverting changes on _line_item.html.erb and keeping changes on_form.html.erb might be the best solution. Thoughts?

Owner
schof commented Apr 19, 2012

@devilcoders what do you think?

Member

I like classes on _form.erb.html but I don;t think that we need to do that on _line_item.erb.html. We can;t remove data-hooks from _line_item.erb.html, data-hooks are for overriding things and if we remove them so users will be very ungry after update. I don't see any problems with using attributes for styling, if css allow that. Also I don't think we should care about IE6. It's gone, and for those less than 1% percent of it's users - sorry guys, please update, it's just about time.

So, I would give green light to _form.erb.html but not for _line_item.erb.html.

Member

I think we can merge this now.

@radar radar closed this in 586dd50 Jun 4, 2012
Member
radar commented Jun 4, 2012

Merged!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment