ui icon indicating copy to clipboard operation
ui copied to clipboard

[Suggestion] Class names namespace to avoid conflicts

Open AndersMad opened this issue 3 years ago • 12 comments

Problem: Class name carousel is too generic - e.g. conflicts with bootstrap or prior own etc.

Solution: Add a fb namespace: e.g. fb- so it becomes fb-carousel__button

Sub problem: The options can change the class name - but e.g. carousel__button seems to be "hardcoded" in Fancybox.js

AndersMad avatar Aug 23 '21 11:08 AndersMad

You already can override all the classnames, it just takes a bit of a work.

In the options, you can do the following

classNames: {
    viewport: "carousel__viewport",
    track: "carousel__track",
    slide: "carousel__slide",
    slideSelected: "is-selected",
},

And alter as needed. You need to manually alter the styles after you do this though

Though it could be made a bit more clear, or maybe just set it in one place (some kind of prefix option maybe, that would get copied across the board?)

Agreed on better prefix for Carousel, that is rather generic, as opposed to Fancybox and Panzoom. Maybe something like fancyapps_carousel, fancyapps_fancybox, etc?

Eldzej avatar Aug 24 '21 13:08 Eldzej

Hi,

I like the idea about the prefix, I think I will add that as an option.

fancyapps avatar Aug 24 '21 13:08 fancyapps

I was more thinking of a generic prefix for all classes, could be fancyapps - but fancyapps_fancybox__slide sounds a bit... verbose :) But a prefix for the most generic would ok too / good enough I spose 👍

I just often find that class names quickly becomes to generic. Targeting the styles can be ok easy like wrap the whole SCSS import in a main class (although a bit bloated) - but that does not help if e.g. bootstrap (and its a bs discussion too I have seen) "bleeds out" across. And also a query selector like ".carousel" will end up "getting too may". I am in an environment where I have no idea what else is there - even in the SCSS stack.

AndersMad avatar Aug 24 '21 13:08 AndersMad

Agreed that is a bit verbose, I was thinking about something that would not conflict with other stuff (just fa sounds like related to FontAwesome, etc.)

Eldzej avatar Aug 24 '21 13:08 Eldzej

Ye, and FontAwesome actually have this SCSS:

$fa-css-prefix: crazyprefix;
.#{$fa-css-prefix}-spin { ...

So if an option - it would go into the SCSS + the JS options (must be same ofc.). Else just go with e.g. fap (fancy app :D )

AndersMad avatar Aug 24 '21 13:08 AndersMad

Honestly, naming was a very difficult decision. All common names are already taken (like Swiper), a random name would also confuse or would be hard to remember, fa- (or f-) prefix could be confused with Facebook or other projects.

On the other hand, Carousel needs very little CSS for basic functionality. You could easily set your custom names using JS options and then just write your CSS that you need.

fancyapps avatar Aug 24 '21 13:08 fancyapps

Naming always is ain't it! ;-) Well, as I am test running your v4 here - my decision is to base it on top of your SCSS files - cause just in case you add some sort of really important thing like css transform, overflow, grid/flex swap etc. that is crucial - then the JS and SCSS package update will take care of that..

AndersMad avatar Aug 24 '21 13:08 AndersMad

+1 for ALL class names used in this project to be namespaced (e.g. fb4-*) by default (or main classname namespaced and all other classes nested under it in scss/css).

Too many other libraries in use now-a-days to use non-namespaced class names. It you are worried about classname length, I would rather see the namespace and a less verbose classname.

Fancybox V4 class names should also be isolated from Fancybox V3 classnames for those migrating to avoid conflicts. Not sure if they are or not--- haven't jumped in that far yet.

.carousel, .compensate-for-scroll are common classnames.

jmalatia avatar Mar 15 '22 18:03 jmalatia

I am going to change naming in v5. Which option would you prefer? Maybe there are other suggestions?

.FancyPanzoom {}        .FancyPanzoom__content {}
.FancyCarousel {}       .FancyCarousel__slide {}
.FancyBox {}            .FancyBox__content {}

OR

.fancy-panzoom {}        .fancy-panzoom__content {}
.fancy-carousel {}       .fancy-carousel__slide {}
.fancybox__container{}   .fancybox__content {}

fancyapps avatar Mar 22 '22 11:03 fancyapps

Personally the latter.

And here is another suggestion - a bit shorter, split of fancybox and with BEM -- for "state" (A):

.fancy_panzoom {}         .fancy_panzoom-content {}
.fancy_carousel {}        .fancy_carousel-slide {}    .fancy_carousel-slide--active {}
.fancy_box-container{}    .fancy_box-content {}

Or even shorter (B):

.f_panz {}          .f_panz-content {}
.f_caro {}          .f_caro-slide {}    .f_caro-slide--active {}
.f_box-container{}  .f_box-content {}

But anything goes :)

AndersMad avatar Mar 23 '22 07:03 AndersMad

Personally I prefer snake_case and/or kebab-case for naming stuff in projects over CamelCase

Maybe slight suggestion for sake of clarity, use fancy as prefix always, including Fancybox (so something like fancy_box, fancy_carousel etc, if going with 🐍).

I like the first suggestion from @AndersMad, the second would probably run into problems of being too confusing with other libraries, as I mentioned before.

Eldzej avatar Mar 26 '22 19:03 Eldzej

Thanks for the suggestions. I will probably choose the simplest solution to add a prefix to all class names. Not sure which one to choose, maybe f- would be fine - simple and short:

.f-panzoom {}	
.f-panzoom__viewport {} 
.f-panzoom__content {}

.f-carousel {}
.f-carousel__track {}
.f-carousel__slide {}

.f-fancybox__container {}
.f-fancybox__backdrop {}
.f-fancybox__slide {}
.f-fancybox__content {}
.f-fancybox__caption {}

fancyapps avatar Mar 29 '22 15:03 fancyapps

Hi,

So, v5 has been released recently and class names are prefixed with f- except for Fancybox.

fancyapps avatar Feb 24 '23 15:02 fancyapps

You might want to revisit this. Many of the fancybox classes are not scoped such as is-hidden. It would be helpful if you would scope everything as to not come into conflict with existing frameworks.

ryanburnette avatar May 09 '23 23:05 ryanburnette