madskristensen / WebEssentials2013

Visual Studio extension
http://vswebessentials.com
Other
945 stars 253 forks source link

Sprite CSS now includes full path #1456

Open nikmd23 opened 10 years ago

nikmd23 commented 10 years ago

The class names generated by WE for CSS sprites used to just be .imgFilename{...}

Now they are .path-to-imgFilname{...}, which is pretty clunky.

Is this a bug, or was this done to solve an issue when there are two images with the same name in different directories?

madskristensen commented 10 years ago

Now that you mention it, this might be by design actually. Is it root relative?

am11 commented 10 years ago

There is this setting under VS > Tools > WE > Sprite: "Use full path for naming identifiers". Set it to false and it will produce decent names (but then you would have to manually take care of naming clashes).

The alternative was to append names with random numbers, which would have impacted the performance badly.

was this done to solve an issue when there are two images with the same name in different directories?

Yes. See https://github.com/madskristensen/WebEssentials2013/issues/913.

nikmd23 commented 10 years ago

Thanks @am11. Flipping that bit made the resultant CSS much more palatable.

@madskristensen it would seem to me that it would be much more sensible to reverse the order of the path so the most significant part is what you read first. Optimally you could then drop any path segments that weren't required make the class name unique.

For example, given this directory structure:

╥ Content
╚╦ Images
 ╠╦ Thumb
 ║╚╦ apple.png
 ║ ╠ orange.png
 ║ ╚ cherry.png
 ╚╦ HighRes
  ╚╦ apple.png
   ╠ orange.png
   ╚ banana.png

I'd love to see this generated, by default:

.apple-thumb {...}
.orange-thumb {...}
.cherry {...}
.apple-highres {...}
.orange-highres {...}
.banana {...}

I see a few advantages to this approach:

  1. Class names are shorter
  2. Class names are easier to read since the "most significant value" is right out front
  3. Given a semi-decent directory naming structure, I'd argue these class names are more semantic
jsassner commented 10 years ago

@nikmd23, i don't agree with your reverse-path naming. I think it's more logic to have the sprite image named as the path they are stored in the filesystem. Since we are getting intellisense on naming lookups, this must be a no-brainer.

GProulx commented 10 years ago

Humm.. I'm currently working on this feature that I expect to be able to complete later this week. Personally I prefer the naming format proposed by @nikmd23 but it's totally an opinion here. Maybe we could add a new settings in the sprite file with the preferred naming convention for selector? (reverse-path, filesystem-path) There is already many settings in this file but I don't think it will be possible to have a consensus on this kind of subject and all arguments are good, depending of your point of view.