Patch (to be ported)
Project:
Twitter Pull
Version:
6.x-1.1
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
4 Aug 2010 at 11:02 UTC
Updated:
29 Jun 2012 at 17:31 UTC
Jump to comment: Most recent file
Comments
Comment #1
kmonty+1
there is no reason for this module to make twitter pull links
color: blue;. No theme uses color: blue; as its default link color... just let people inherent their default link colors.Comment #2
irakli commentedI am kinder to themers. All (I mean ALL) styles in the css file attached by default are for the descendants of div.tweets-pulled-listing element. When you create your TPL file, change the CSS classname on the main div from "tweet-pulled-listing" to something else and your template won't pick-up any of the default styles.
Comment #3
kmontyThat isn't how modules should be constructed to work (notice why no other module requires you to override the default tpl?). Do you think anyone wants their twitter links to be color: blue;
Let twitter_pull inherent theme defaults, not define their own defaults.
Comment #4
irakli commentedOK, let's not make blanket statements.
All these said, and re-reading the comments, looks like blue link color is the cause of most anger here. I don't have any problem killing that one. I am going to have to edit the title of the issue, though, because current title does not reflect what needs to be done here. There's important difference between not providing any style and not liking that the default link is blue.
Default styles (layout etc.) stay, link color goes away and will be inherited from the theme.
Thank you.
Comment #5
kmontyThis also should be pulled out for the same reason:
Comment #6
Marko B commentedI agree, link color and stayling altogether should be inherited from active theme, there is no reason to be otherwise in most cases.
Comment #7
mstrelan commented+1 on the removal of blue links. As for other modules, I cannot think of one that actually sets the color of an element, unless it is to a neutral grey sort of color.
I'm actually not particularly worried about the tweet-time being silver. The reason I say this is that in a large number of scenarios it is very likely that the tweet-time link won't look like a link, and so themers will have to specifically target that link anyway if they don't like the default color. Having said this, to cover the other crowd who want it to look like a link well then it should be unstyled by the module.
Comment #8
mrfelton commentedDefinitely think the link should not be forced into blue. I can see why you may want to float the image to the left in the default styles, but I can't see any reason why any other style properties should be set. Definitely not the link color, and probably not the color of the date. Bold on the author name, ok maybe. Colors should be inherited from the theme, otherwise 99 times out of 10, the twitter feed block just looks strage with different colored links to everything else on the page.
One other thing, although probably out of scope for this ticket - you probably shouldn't use hook_init() load the styles. They are being loaded for every single page of the site. It would be better to only load the styles when a twitter feed is actually being displayed.
Comment #9
mstrelan commentedLoading CSS in hook_init() helps to minimise the number of aggregated CSS files are generated. But if CSS aggregation is disabled then it increases the number of CSS files loaded by the browser. In my opinion modules should check if preprocess_css is TRUE and if so load CSS in hook_init() otherwise on demand. I posted my thoughts on this against the Date module #369159: Only add css when required.
Probably the easiest way would be to have a function MYMODULE_load_css() with a static boolean variable $loaded. In hook_init() check if preprocess_css is TRUE and only if it is TRUE call MYMODULE_load_css(). Then every time you need the CSS on demand call MYMODULE_load_css(). That function will check if its already loaded, and if it is then it will do nothing.
Comment #10
star-szrSubscribe.
Comment #11
jurriaanroelofs commenteda module should _never_ add styling to it's output.
structure and layout, yes if you must.
colors, borders, font-weights, never.
+1 for removal.
Comment #12
star-szrHere's a patch against 7.x-1.x-dev (applies to 7.x-1.0-rc1, 6.x-1.x-dev, 6.x-1.2 as well). The patch removes all the colors from the stylesheet. Hopefully this very minor change can get committed and we can all move on with our lives.
Comment #13
artfulrobot commented+1 no blue links (sympathies with author who points out that these styles are only for use with the default theme, but event then, the default theme should integrate nicer and not set blue).
+1 only load styles when needed. As the author points out, these styles are for the default theme only. If you're not using the default theme, this is unnecessary bloat whether it's aggregated or not.
(grateful to author for writing and sharing great module!)
Comment #14
mortendk commented+1 for killing of the link colour - was really not a nice surprise when that showed up
atm im killing direcly all style instead from in otherways a good module with the foad
Comment #15
jec006 commentedCommitted here to 7: http://drupalcode.org/project/twitter_pull.git/commit/c8993e8
Thanks guys, especially Cottser
Comment #16
star-szrThanks @jec006! Is the patch (to be ported) for D6? The patch I posted should apply cleanly to 6.x-1.x as well since the stylesheets were the same.