Closed ywarnier closed 1 year ago
I'm considering more and more to just drop the complete extension and have a special function to deal with adding ".png" or ".svg" when no extension is given.
Also, change icon names in the system_template table.
At the moment @aragonc is using harcoded icon names for tools in the master branch (instead of using c_tool.image), but that will have to be changed because it blocks the definition of custom icons, unless we consider the c_tool.image field to be empty until a customization is used and then it is filled with some value, which also seems fine except the process of preparing the array of tools might be much more complex.
After discussing this issue with @jmontoyaa, we decided to remove the icon column from the c_tool table and use the tool name defined in the "tool" table, adding a .png or .svg suffix. This will require renaming the icons (which I can do myself once the column is removed). Julio will review the course home scripts first
Is there any way to use a icon loaded from external link, for example to load the icon for LTI tools?
I moved the icons to a folder here:
https://github.com/chamilo/chamilo-lms/tree/master/public/img/tools
Images are not used anymore, instead an mdi icon is used. The icons are set here:
https://github.com/chamilo/chamilo-lms/blob/master/src/CoreBundle/Resources/config/tools.yml
Icons now all defined in tool classes in https://github.com/chamilo/chamilo-lms/tree/master/src/CoreBundle/Tool
Current behavior
The c_tool records created by Chamilo all use .gif icon references (except for plugins). In the "big-activity" view, the course homepage view transforms these .gif in .png, but this is an added process (see https://github.com/chamilo/chamilo-lms/blob/1.11.x/main/inc/lib/course_home.lib.php#L1037).
Expected behavior
We started to remove all .gif images, so it would make sense to move all these references to .png.
In a sense, and considering we are often still cutting the image names to add the "_na" suffix for gray icons, it would make sense to only keep the name of the icon without any extension, but that would make other things (like the return_icon function) more complicated and it is likely it would not result in a performance gain overall.
How to proceed
This change involves an upgrade procedure. As such, it should be included in the upgrade process to get to the next major version (2.0). Also, it should not affect c_tool records that are not part of the default Chamilo. As such, we should provide a whiltelist of the only icons which need modifications and do something like this:
After that, the main/course_home/ scripts would probably need to be updated to reflect the change to PNG, which implies adding the size of the icon as well (which is currently not part of the image path, as there is only one version of the gifs). And of course updating the course_home.lib.php reference above to remove the str_replace() in getToolIcon()