Skip to content
This repository

Removed -moz-border-radius and -mox-box-shadow as Firefox 3.6 reached EOL. #95

Merged
merged 1 commit into from almost 2 years ago

8 participants

Frank Phil LaPier Léo Renaud-Allaire Jasdeep Jaitla Hengjie Reda Lemeden Sascha Michael Trinkaus
Frank

Yay for the gradual death of vendor prefixes.

Here is Mozilla's official announcement, https://blog.mozilla.org/futurereleases/2012/03/23/upcoming-firefox-support-changes/

Phil LaPier
Collaborator
plapier commented May 10, 2012

This is great to hear, i've done some research on this and it seems firefox 3.6 only has a 3.1% total usage rate amongst total firefox users (35.8%). I think it's safe to remove.
Source: http://www.w3schools.com/browsers/browsers_firefox.asp

But it also begs the question, is it safe to drop the webkit vendor prefixes for border-radius? I think the only real use is for iOS Safari 3.2. And even then, if you know your audience uses that version, you can create your own border-radius mixin locally.
Source: http://caniuse.com/#feat=border-radius

Can and should we deprecate the border-radius mixin entirely?

Frank

+1 from me. It makes complete sense to deprecate mixins that are no longer needed. That is why I love Bourbon so much, it encourages the use of pure CSS where possible and keeps the library super lightweight.

Léo Renaud-Allaire

+1 from me too. That being said, I would not rely too much on W3 Schools browsers stats, as the data is from w3schools.com users, wich is not representative of the average user.

Net Market Share shows 2.43% of global users are using Firefox 3.6

Stats Counter shows 2.61%

Still, that's a pretty low percentage and I agree the mixin could be deprecated.

Phil LaPier plapier merged commit e18d11a into from May 11, 2012
Phil LaPier plapier closed this May 11, 2012
Jasdeep Jaitla

my only problem with removing it is that you have to go through every sass file of every project and remove it there, or add your own mixin so that you don't have to replace them all... ? am I misunderstanding removal?

Phil LaPier
Collaborator

@scalabl3 You are misunderstanding removal. The bourbon mixin will take care of it for you. When you upgrade the gem and recompile your sass, the -moz- vendor prefixes for border-radius and box-shadow will be removed automatically. There is little to no work required from the user.

Jasdeep Jaitla

@plapier ah, sorry my bad, just removal of the browser-vendor prefix, not removal of the mixin! When I read it on the Bourbon page, i didn't read what was being removed correctly... I thought it was talking about the mixin. I need more coffee. :)

It's the lingo: Deprecation Warning: The border-radius mixin is deprecated and will be removed in the next major version release.

it says the mixin will be removed....

Léo Renaud-Allaire

Actually there has been some discussion about removing the whole mixin, as suggested by @plapier. Is there any way to slowly deprecate the whole mixin by adding some kind of warning or by making sure it doesn't just break existing projects?

Phil LaPier
Collaborator

I just double checked, and it looks like the @border-radius mixin is now deprecated, but the mixin will remain until the next major version release, where it will be removed. Therefore on release of Bourbon 3.0, you will need to change all your @include border-radius(*); to standard css border-radius: *;

Jasdeep Jaitla

@plapier ack, that sucks actually, i mean you can add your own mixin with the same parameters but it just means more headache to go through all those projects for everyone using it... Is there a big benefit for Bourbon to remove it completely?

Phil LaPier
Collaborator

When I deprecated the mixin I added a warning: https://github.com/thoughtbot/bourbon/blob/master/app/assets/stylesheets/css3/_border-radius.scss

The mixin could remain in 3.0+, but output only an official css3 declaration and no vendor-prefixes. That way it doesn't break a bunch of projects. I'm not sure that's best practice though?

Jasdeep Jaitla

@plapier hehe, i think best practice depends on what you prioritize, this would be in the realm of "backward compatibility" for the sake of all us users out there having to do a lot of work to remove it :)

However, from the Bourbon perspective, the mixin no longer is a shortcut for anything, and that is the purpose of Bourbon (to maintain compatibility across browsers and having single statements translate into vendor-specific ones).

I tend towards the former myself... it doesn't hurt to have the mixin because it doesn't cause any problems, but removing it could if they aren't aware when they do a bundle update...

Léo Renaud-Allaire

The thing is, almost every Bourbon's mixin that only adds vendor-prefixed properties will have to be deprecated at some point. If not, it will slowly go from a useful tool to something that adds another layer over things that don't need it.

Maybe there could be an optional file containing all the deprecated mixins that now outputs only the official css3 declaration. That way, if someone simply can't go over all of his projects to remove the calls to deprecated mixins, that person can simply load that file in the main _bourbon.scss file.

What do you think of that approach?

Jasdeep Jaitla

+1 @renaudleo

so you could have:

@import bourbon
@import bourbon-deprecated

that would still mean you have to change your project code, but it's minimized...

Phil LaPier
Collaborator

So the user would only import the deprecated mixins if they chose to? Something like this?

@import 'bourbon/bourbon';
@import 'bourbon/deprecated-mixins';
Léo Renaud-Allaire

Yes, I think we still want the default behavior to be the most up-to-date one, but this approach could provide an easy way for people to deal with deprecated mixins in one step, as we know they're will be more in the future.

Jasdeep Jaitla

I agree, this is a good solution. It's not as big a deal to add one line of code, vs going through all your sass/scss files and replacing statements in every project, or alternatively making your own mixin file to replace deprecated ones and import that in every project.

Hengjie

Yes, that approach seems like the best compromise.

Reda Lemeden
Collaborator

I don't see how entirely removing deprecated mixins could be a problem. Those using the gem can simply keep using the version they used the first time. Other can refrain from updating the Bourbon folder or simply do a global find and replace in case they want to take advantage of new bourbon niceties. Not to mention that sooner or later, we'll need to start dropping features anyway to keep things in control and promote good CSS practices.

I think a hybrid approach would be grouping the soon-to-be-deprecated mixins in a separate .scss file as suggested above, with a deprecation @warn, then entirely removing them in the next minor version bump or two.

Sascha Michael Trinkaus

I think the suggestions to import deprecated mixins to revive old vendor-prefixed properties from the dead is a good solution for people who maintain projects over a long time period or want to go back and simply change stuff on older ones. What's the current status on this? Will it be implemented like that?

Phil LaPier
Collaborator

@saschamt I'm thinking it might. I think the grouped mixins in a single soon-to-be-deprecated file is a good idea. Deprecated code will get dropped for new major version releases (2.x -> 3.0). This is pretty standard for open-source projects.

Tommaso Visconti tommyblue referenced this pull request in stefanoverna/activeadmin-dragonfly December 08, 2012
Closed

bourbon border-radius mixin is deprecated #1

Phil LaPier plapier referenced this pull request February 19, 2013
Closed

Deprecate box-shadow #156

Matt Goins mjg2203 referenced this pull request from a commit in mjg2203/edx-platform-seas July 08, 2013
David Baumgold Changed `@include border-radius()` to `border-radius`
The official border-radius mixins were deprecated and removed in Bourbon 3.0.
thoughtbot/bourbon#95
4137483
Jessie A. Young jessieay referenced this pull request from a commit November 26, 2013
Commit has since been removed from the repository and is no longer available.
Jessie A. Young jessieay referenced this pull request from a commit November 26, 2013
Commit has since been removed from the repository and is no longer available.

I'm slightly confused by the text at http://bourbon.io/docs/#border-radius - First it says that these (I presume it refers to the ones in the example) "are supported in Bourbon v3.0+". Yet further down we read "The official border-radius mixins were deprecated and removed in Bourbon 3.0". I'm not sure if this text references a mixing named just border-radius rather than the ones in the example above, but whatever the case is, this text is very confusing :P

Perhaps it can be clarified?

Phil LaPier
Collaborator

@rawtaz @include border-radius( val ); has been deprecated and removed from Bourbon.

@include border-top-radius( val ); is a shorthand mixin and thus still exists in Bourbon.

You can view the rest of the shorthand mixins here or in the Docs.

@plapier Thanks. I think the confusing part is the wording "the official border-radius mixins were deprecated", which at least the way I read it seem to imply multiple mixins were removed, presumably then related to border-radius since this single one cannot be an example of multiple things having been removed.

I mean, it sounds more like some mixins in that category (e.g. border-foo-radius and border-bar-radius) were deprecated, rather than border-radius.

If it was just one mixin that was removed, i.e. border-radius, then perhaps it makes more sense to write "The official border-radius mixin was deprecated .."?

Phil LaPier
Collaborator

@rawtaz Updated the copy. Thanks for suggesting a change. http://bourbon.io/docs/#border-radius
You may need to hard refresh.

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

Showing 1 unique commit by 1 author.

Apr 29, 2012
Removed -moz-border-radius and -mox-box-shadow as FF 3.6 reached EOL. e18d11a
This page is out of date. Refresh to see the latest.
9  app/assets/stylesheets/css3/_border-radius.scss
... ...
@@ -1,34 +1,25 @@
1 1
 @mixin border-radius ($radii) {
2 2
   -webkit-border-radius: $radii;
3  
-     -moz-border-radius: $radii;
4 3
           border-radius: $radii;
5 4
 }
6 5
 
7 6
 @mixin border-top-left-radius($radii) {
8 7
   -webkit-border-top-left-radius: $radii;
9  
-     -moz-border-top-left-radius: $radii;
10  
-      -moz-border-radius-topleft: $radii;
11 8
           border-top-left-radius: $radii;
12 9
 }
13 10
 
14 11
 @mixin border-top-right-radius($radii) {
15 12
   -webkit-border-top-right-radius: $radii;
16  
-     -moz-border-top-right-radius: $radii;
17  
-      -moz-border-radius-topright: $radii;
18 13
           border-top-right-radius: $radii;
19 14
 }
20 15
 
21 16
 @mixin border-bottom-left-radius($radii) {
22 17
   -webkit-border-bottom-left-radius: $radii;
23  
-     -moz-border-bottom-left-radius: $radii;
24  
-      -moz-border-radius-bottomleft: $radii;
25 18
           border-bottom-left-radius: $radii;
26 19
 }
27 20
 
28 21
 @mixin border-bottom-right-radius($radii) {
29 22
   -webkit-border-bottom-right-radius: $radii;
30  
-     -moz-border-bottom-right-radius: $radii;
31  
-      -moz-border-radius-bottomright: $radii;
32 23
           border-bottom-right-radius: $radii;
33 24
 }
34 25
 
1  app/assets/stylesheets/css3/_box-shadow.scss
@@ -9,6 +9,5 @@
9 9
                    $shadow-5, $shadow-6, $shadow-7, $shadow-8, $shadow-9);
10 10
 
11 11
   -webkit-box-shadow: $full;
12  
-     -moz-box-shadow: $full;
13 12
           box-shadow: $full;
14 13
 }
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.