Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.The 'default formatting' provided by trip_search_html_head() is IMHO unnecessary. It not only adds to page load time and filesize (because it's not cached as an external css would be), it's also redundant as it's declarations would normally be overwritten by the theme designer in his sites css.
Because it's inline css, the theme designer also has to be wary of the order of cascade and to ensure his stylesheets load in sequence so that he can override the declarations made here. Not all themes make it easy to control the order of objects loaded into the <head>...
Besides, why define default values if they're _meant_ to be changed? Defining the labels or classnames used? Yes I can see the sense, but values should be suggested in the readme.
There may well be other valid or future uses for the function trip_search_html_head(), so I am loath to remove it.
I vote we remove the css to the readme.txt either as an example, or just as a note to add the various class names to the sites theme css. This patch is a step in that direction, in that it simply:
- comments out the above function
- removes the css declarations
ps. Should I patch the readme too? Please say 'no'... ;)
| Comment | File | Size | Author |
|---|---|---|---|
| trip_search-nocss.patch | 1.19 KB | marky |
Comments
Comment #1
moshe weitzman commentedA few responses ...
- the point of specifying CSS in the module is to give admins a pretty default view. If they don't like the default view, they are welcome to change it.
- If you want move the default CSS to an external stylesheet, thats OK with me
- All themes import their own CSS after drupal_get_htmk_head(). Thus themes already can easily override module supplied CSS.
Comment #2
marky commented"- the point of specifying CSS in the module is to give admins a pretty default view. If they don't like the default view, they are welcome to change it."
Yep, I understood the reasoning, that's why I can see its flaws ;)
To do so they currently have 2 options:
1. hack the module directly
2. double their css declarations
"- If you want move the default CSS to an external stylesheet, thats OK with me"
Exactly. It should be external, and optional. Ideally there would be a note in the readme indicating the classnames used by the module so the declarations can be integrated with the sites theme css. I don't use the trip_search block - the module gives me that choice. But I _always_ get it's style block in the head whether I use it or not.
"- All themes import their own CSS after drupal_get_htmk_head(). Thus themes already can easily override module supplied CSS."
The point remains that the css is unnecessarily declared twice. For each declaration the browser has double the work to do before it can render the page. Ok, we're talking microseconds here, but people often go to great lengths to compact their css (#fff instead of #ffffff, margin:0, etc) in an effort to shave a few bytes off. Using this module in it's present state requires you to duplicate code unless you accept the default style. Worse still, if you're not using the block the duplication is completely pointless...