developer.jelix.org is not used any more and exists only for history. Post new tickets on the Github account.
developer.jelix.org n'est plus utilisée, et existe uniquement pour son historique. Postez les nouveaux tickets sur le compte github.

Opened 13 years ago

Closed 13 years ago

#348 closed enhancement (fixed)

improve addCssLink usability for IE

Reported by: bibo Owned by: laurentj
Priority: normal Milestone: Jelix 1.0 RC1
Component: jelix:core response Version: 1.0 beta 3.1
Severity: minor Keywords:
Cc: Blocked By:
Blocking: Documentation needed:
Hosting Provider: Php version:

Description

addCssLink ouput <!--[if IE]> if $forIE=true.

That could be improved as nowadays, IE 7 respects standards much more and therefore there is a need for <!--[if lt IE 7]> to throw some shitty css to IE 6 and below.

I propose this new signature for addCSSLink.

    final public function addCSSLink ($src, $params=array (), $forIE=false, string $ieCondition = '' )

internally $ieCondition could be stored as a special _CSSIELink. And used for the correct output.

shall i add that this could apply equally to addJSLink. if this enhancement is accepted, i'll be glad to push a patch.

Attachments (1)

jelix-trunk-#348.patch (4.4 KB) - added by bibo 13 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 13 years ago by laurentj

I'm waiting for your patch. Don't forget to modify also the meta.html.php plugin for jtpl, so we could do {meta_html cssie6 ..} or cssie7 etc.. For the API of addCssLink, $forIe could be 'true' (all IE version), or 6, 7, etc.. Same behavior for addJsLink.

comment:2 follow-up: Changed 13 years ago by bballizlife

I really don't want addCSSLink() to become such a hacked thing. The main goal of that method is to add a CSS file. Simply.

If we have more customs needs, there is the addHeadContent() method for that.

comment:3 in reply to: ↑ 2 Changed 13 years ago by bibo

Replying to bballizlife:

I really don't want addCSSLink() to become such a hacked thing. The main goal of that method is to add a CSS file. Simply.

Ok but if I follow you, $forIE argument should not exist then as it is not really useful that way.

It's up to you (bballiz and laurent), about the option to follow.

comment:4 Changed 13 years ago by laurentj

@bibo : After a discussion on IRC, we are OK for a patch.

comment:5 Changed 13 years ago by bibo

First patch. I slightly changed the logic behind _CSSIELink.

this patch adds a conditionnal comment for every ie-specific CSS.

on the template side, i don't see how i could define a generic template to reflect the enhanced addCSSLink. Maybe, i could just put a cssie6 or cssltie7, if it is considered useful enough to exist. tell me about that.

comment:6 Changed 13 years ago by laurentj

Maybe, i could just put a cssie6 or cssltie7, if it is considered useful enough to exist.

Yes it is enough.

Changed 13 years ago by bibo

comment:7 Changed 13 years ago by bibo

I've updated the patch. now, it also adds those values to meta tpl plugin.

meta cssie7 meta cssltie7 meta ccsthemeie meta cssthemeie7 meta cssthemeltie7

this patch also fixes ticket #347

comment:8 Changed 13 years ago by laurentj

  • Milestone set to Jelix 1.0 RC1
  • Resolution set to fixed
  • Status changed from new to closed

Patch landed in the trunk. Thank you for the patch !

Note: See TracTickets for help on using tickets.