Skip to content

[ticket/15538] Icon usage twig function#5489

Closed
ghost wants to merge 2 commits intomasterfrom
unknown repository
Closed

[ticket/15538] Icon usage twig function#5489
ghost wants to merge 2 commits intomasterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Dec 27, 2018

PHPBB3-15538

Checklist:

  • Correct branch: master for new features; 3.2.x for fixes
  • Tests pass
  • Code follows coding guidelines: master and 3.2.x
  • Commit follows commit message format

Tracker ticket (set the ticket ID to your ticket ID):
https://tracker.phpbb.com/browse/PHPBB3-15538


This is for the Chameleon Theme, to help out @hanakin.
No PNG icons will be shipped initially with the theme, but SVG's will be.
This is not to say that other style authors do not want to use PNG's, so there is support for png aswell.
The assumption is that icons will be located in

  • theme/icons/png/ some_icon.png
  • theme/icons/svg/ some_icon.svg

In regards to loading times, I do not know what is faster:

  • hard coding html strings for the various icons and caching those
  • rendering the icon.html through the environment

This also goes for the svg's <path d="">. Cause the svg's will mostlikely be 'stand-alone' versions of the icon (<svg>..</svg>) and we only want to use the the d="" part from the <path>. Anyways, I am not sure what the fastest way is to get the contents of the svg file:

  • Through the environment render()
  • Through the filesystem?
  • ... ?

Moreover the svg <path d="">, I couldn't quite find it online yet but what are the exact allowed characters in that specific attribute? I am currently using '/d="([a-zA-Z0-9., ]+)"/' as a regex. Anything that needs to be included/excluded?

The current icon.html file needs work, was just a quick mock up to get something to be displayed.

@ghost
Copy link
Copy Markdown
Author

ghost commented Dec 27, 2018

!set WIP 🚧

@@ -0,0 +1,10 @@
{% if TYPE == 'png' %}
<img class="icon png-{{ ICON ~ S_CLASSES }}" src="{{ T_PATH }}"{% if TITLE %} alt="{{ TITLE }}"{% endif %} style="background: black;">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats with the hardcoded style?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah my bad, I downloaded the white png 😵 couldn't see it, like I said, the entire html file needs work as it's just a mock. Decided that that will be for you.
First want to make sure the PHP and loading times are correct.

{% if TYPE == 'png' %}
<img class="icon png-{{ ICON ~ S_CLASSES }}" src="{{ T_PATH }}"{% if TITLE %} alt="{{ TITLE }}"{% endif %} style="background: black;">
{% elseif TYPE == 'font' %}
<i class="icon fa-{{ ICON ~ S_CLASSES }}" aria-hidden="true"{{ S_ATTRIBUTES }}></i>{% if TITLE %}<span{% if S_HIDDEN %} class="sr-only"{% endif %}>{{ TITLE }}</span>{% endif %}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aria hidden should only be there if title else its should be false

{% elseif TYPE == 'font' %}
<i class="icon fa-{{ ICON ~ S_CLASSES }}" aria-hidden="true"{{ S_ATTRIBUTES }}></i>{% if TITLE %}<span{% if S_HIDDEN %} class="sr-only"{% endif %}>{{ TITLE }}</span>{% endif %}
{% elseif TYPE == 'svg' %}
<svg class="icon svg-{{ ICON ~ S_CLASSES }}"{% if TITLE %} aria-hidden="true" aria-labeled-by="{{ TITLE_CLEAN }}"{% endif %} xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" width="24" height="24" viewBox="0 0 24 24">
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not need all of the extra stuff here you can remove

xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1"  width="24" height="24"

@hanakin
Copy link
Copy Markdown
Member

hanakin commented Dec 27, 2018

@CHItA @Nicofuma you insights for the best way to implement this would probably help as well. IIRC one of you had an idea for handling this

@hanakin hanakin changed the title [ticket/15538] Chameleon icons [ticket/15538] Icon usage twig function Dec 27, 2018
@hanakin
Copy link
Copy Markdown
Member

hanakin commented Jan 2, 2019

have a look at some of the code from this old pr https://github.com/phpbb/phpbb/pull/4765/files might help with testing and what not

@hanakin
Copy link
Copy Markdown
Member

hanakin commented Feb 28, 2019

@mrgoldy where did we leave of with this? should probably create some tests. We will need to test it in all locations/possible uses that is to say I want to use it for icons, imagesets, contact icons, forum icons, smiles

the last one is a maybe

but this means we need to be able to specify in the case of forum icons a different path as they are in the files folder same with the smiles.

@hanakin hanakin added the Chameleon 🦄 New Default Theme label Feb 28, 2019
@ghost ghost closed this Mar 5, 2019
@ghost ghost deleted the ticket/15538 branch March 5, 2019 22:11
@ghost ghost mentioned this pull request Mar 8, 2019
4 tasks
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants