search

synapsestudios / kohana-notices

Notices module for Kohana 3.x
34 stars 6 forks source link

notice type image is a style, not a piece of content #7

Open zeelot opened 14 years ago

zeelot commented 14 years ago

you should not have an tag in your notice output for the type of notice... this is equivalent to the bullets in an unordered list! It's purely a style of the notice! This also simplifies the adding of the image... as you can force the convention of the css and img folders being next to each other and use relative paths .error images will always be ../img/error.png (or whatever convention you want to set)

jeremeamia commented 14 years ago

That would be nice... but I'll probably never change this. No... not unless I am feeling really motivated.

zeelot commented 14 years ago

how would you feel about the close button also being css? with text 'Close' in the html... I'll take the issue

jeremeamia commented 14 years ago

I'm certainly not opposed to either of the images being a part of the CSS. It definitely cleans up the HTML. I don't remember why I didn't do it like this to begin with. It's possible there was a reason... but, I certainly don't remember what it would be.

zeelot commented 14 years ago

I made the change in a bugfix branch for now... I would love a code(css) review on this from one of you guys with more css knowledge (that's probably all 3 of you, jeremy, alan, and david)

here's the commit: http://github.com/synapsestudios/kohana-notices/commit/17c4e6a093fc3a1f77fad8d28c0b57801c9e49b5

it would seem that if the image is missing, the close button basically becomes invisible... but with the media no longer being a config you need to worry about because of the relative paths, then I don't think this is a big issue... am I correct?