? Pending

User tests: Successful: Unsuccessful:

avatar htmgarcia
htmgarcia
19 Sep 2018

Pull Request for Issue NA .

Summary of Changes

The Content - Vote plugin currently uses png images to display the rating stars in articles. Let's take advantage of Font Awesome and use icons instead.
lorem_ipsum_rocks___3_

Testing Instructions

Enable Content - Vote plugin for articles
Preview in frontend

Expected result

Actual result

Documentation Changes Required

avatar htmgarcia htmgarcia - open - 19 Sep 2018
avatar htmgarcia htmgarcia - change - 19 Sep 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Sep 2018
Category Front End Plugins
avatar htmgarcia htmgarcia - change - 19 Sep 2018
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - edited - 19 Sep 2018
avatar Quy Quy - change - 19 Sep 2018
Title
Switch rating stars from png images to icons
[4.0] Switch rating stars from png images to icons
avatar brianteeman
brianteeman - comment - 19 Sep 2018

This adds a hard requirement to use font awesome in all front end templates. Is that really desirable?

avatar mbabker
mbabker - comment - 19 Sep 2018

It's an overridable layout (the whole voting capability should still just be inlined into com_content but ¯\_(ツ)_/¯)

avatar brianteeman
brianteeman - comment - 19 Sep 2018

it may well be right now but one of the aims was to remove that requirement not add to it

avatar htmgarcia
htmgarcia - comment - 19 Sep 2018

@brianteeman this "let's not stick with Bootstrap" but at the same time "let's use it partially" approach is inconsistent. I find impossible to know where is fine and where is not to make use of dependencies like BS4 (and now same case with Font Awesome).


A clear guidance on this matter would be useful.

avatar mbabker
mbabker - comment - 19 Sep 2018

My two cents. We're never going to have a core UI that is 100% decoupled from any framework. So, as long as we have all this "framework coupling" in overridable layouts, at this point I'm leaning toward "who cares, replace it if you want".

avatar brianteeman
brianteeman - comment - 19 Sep 2018

I am just repeating what "powers that be" have said about frontend decoupling - not giving an opinion as thatis too dangerous

avatar htmgarcia
htmgarcia - comment - 19 Sep 2018

Keeping consistency is a good practice, and also is my personal opinion.

Regards

avatar htmgarcia htmgarcia - change - 19 Sep 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-09-19 20:34:42
Closed_By htmgarcia
avatar htmgarcia htmgarcia - close - 19 Sep 2018
avatar laoneo
laoneo - comment - 20 Sep 2018

I would rather go with joomla/40-backend-template#441. I'v used a similar technique in DPCalendar. Like that it is possible for a template to override icons. If there is interest, I can migrate the pr here.

avatar brianteeman
brianteeman - comment - 20 Sep 2018

@laoneo is that needed? This is a front-end layout that can already be overridden

avatar laoneo
laoneo - comment - 20 Sep 2018

Icons have always been a pain in Joomla. There should be an easy way to change them and to have a strategy which serves the core and extension devs.

Making always an override of every view when you want to change the icon set sounds for me not optimal. Keep in mind we have forms, etc. with icons. Having a common layout where you can pass a string and it returns an icon is for me the way to go.

avatar mbabker
mbabker - comment - 20 Sep 2018

Having a common layout where you can pass a string and it returns an icon is for me the way to go.

Explain this bit in more detail. Because right now I'm afraid you're going to have this as a layout file, which would be about as crappy as that endtab.php file:

<?php
$icon = $displayData['icon'];
?>
<span class="<?php echo $icon; ?>"></span>
avatar laoneo
laoneo - comment - 20 Sep 2018

We do load them inline in a JLayout. So the template dev can override the layout itself or place the required icons in the template folder itself.

Add a Comment

Login with GitHub to post a comment