Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.0][RFC] svg icons instead of the ugly gif #21410

Closed
wants to merge 1 commit into from
Closed

[4.0][RFC] svg icons instead of the ugly gif #21410

wants to merge 1 commit into from

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

A dead simple custom element that converts a country code (eg gb) to a flag.
Accepts:

  • Country: a country abbr, eg us, gb, gr
  • Path: the path for the svg file (probably this shouldn't be publically accessible XSS...)

Testing Instructions

Enable multilingual, etc, check the flag...

Expected result

screenshot 2018-08-05 at 01 32 11

screenshot 2018-08-05 at 01 32 31

Actual result

Documentation Changes Required

@joomla-cms-bot joomla-cms-bot added PR-4.0-dev RFC Request for Comment labels Aug 4, 2018
@brianteeman
Copy link
Contributor

As these are not languages flags but country flags (correctly) maybe the element should have a more appropriate name

@ghost
Copy link

ghost commented Aug 5, 2018

at Tester: this Pull Request need npm, it can't be tested by Patchtester.

@HLeithner
Copy link
Member

It seams that the images get loaded from a remote site on visit? If so can we plz ship it with joomla?

@infograf768
Copy link
Member

Switcher:
Weird images size (14x14). Typical flag ratio is 18 x 12
No more horizontal display when not using dropdown for flags or no flags + no Full Language Names

Content languages image select not taken into account any more. Try to use ta for example which is a custom gif made specially to not represent a specific country.

Basically imho a wrong good idea.

@dgrammatiko
Copy link
Contributor Author

Basically imho a wrong good idea.

So lets ship those ugly gifs ?
The idea here is to see what I've missed here and transition to svgs not saying this doesn't work lets stick to the 10 year old crappy gifs...

@dgrammatiko
Copy link
Contributor Author

maybe the element should have a more appropriate name

Makes sense

@dgrammatiko
Copy link
Contributor Author

If so can we plz ship it with joomla?

That's the idea, if you see the code there is a todo for this:
// @todo use images from: https://github.com/lipis/flag-icon-css

@brianteeman
Copy link
Contributor

From an a11y perspective you now have an "image" as a link with no description of the link. Which means that a screen reader can only announce the link and not the meaning of the link

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Aug 5, 2018

👎 for CE.

@dgrammatiko
Copy link
Contributor Author

👎 for CE

Can I ask why?

@infograf768
Copy link
Member

lets stick to the 10 year old crappy gifs...

why would a gif be crappy? Because they are 10 years (or more) old ?

if anything that was 10 years old was crappy, we are all crappy here... 😸
Although, to prevent any undesired comments, i do accept personnaly to be qualified as crappy... 7 times after calculation).

serious. switching to that type of svg (depending on lang country code) just prevents to add custom images when we (or an admin) need it. The example above with tamil is clear enough. Esperanto would be another one I guess. And maybe others too as i did not test all situations (like swahili).

Another interesting aspect to consider (and maybe unrelated to this) is the fact that media is now compiled by npm. How would someone add/change svg in one’s site only.

@Fedik
Copy link
Member

Fedik commented Aug 5, 2018

I see two issue here, how it can be improved:

  1. I know you love JavaScript, but this can be easily done on pure CSS, all you need is:
    <span style="background-image: url(blabla.svg)"></span> + little CSS in mod_languages.css
  2. Inline style make it harder for frontender to customize, I meant width and height

@dgrammatiko
Copy link
Contributor Author

this can be easily done on pure CSS

True, but then we miss the lazyloading feature that JS gives us :) This was done like this for performance (eg take all the images out of the critical path!!!)

why would a gif be crappy? Because they are 10 years (or more) old

Crappy because gifs are not vector images, eg you will never be able to scale up a gif (especially those Joomla provides for the languages) without bad results...

just prevents to add custom images

Wrong

How would someone add/change svg in one’s site only

The same way they do in J3, by creating a file in template/image (this is not implemented as this is an RFC but I don't mean to break the known patterns here)

@HLeithner
Copy link
Member

If we talk about Performance it would be better if the flag is embeded instead of lazy loaded. Most? flags only have 300bytes uncompressed.

A tcp connection with a new request could be very expensive.

@brianteeman
Copy link
Contributor

Definitely need to see some stats to back up the performance gain

@HLeithner
Copy link
Member

I like the svg images but see no need for js here.

@dgrammatiko
Copy link
Contributor Author

Definitely need to see some stats to back up the performance gain

You don't have to if you understand the basics of how browsers work:

  • tags with attribute src invoke immediately a fetch (exception the defer on script tag)
  • the fetch happens in the critical path, eg delays the rendering

So what is different here?
The initial rendering has 0 tags with the source (even the custom element is lazy loaded). We might end up with the same number of http requests BUT those are not blocking the rendering so it's pretty obvious we do have some gains in performance. Of course, as long as you load a stylesheet like Bootstrap in the critical path, any performance gains from solutions like this are totally wasted in the big picture that we're doing asset management in the client side totally wrong. All these were explained in detail in my talk in JAB2018 were I presented a website with a mark 100% in Google's lighthouse (3g/mobile/slowdown) https://www.youtube.com/watch?v=Hg_ATQEl9_U

@dgrammatiko
Copy link
Contributor Author

If we talk about Performance it would be better if the flag is embedded instead of lazy loaded

That idea was turned down by the release leader: joomla/40-backend-template#441 but I guess someone else can pursue that path.

After years of me trying to replace the gifs with svgs, I still see strong opposition therefore I'm gonna close this PR. Personally, I probably can do whatever I need in my websites, it's just that one more module out of the box is useless for anyone that wants to deliver a half-decent website...

@dgrammatiko dgrammatiko closed this Aug 5, 2018
@dgrammatiko dgrammatiko deleted the custom-elements/4.0-dev/flags branch August 5, 2018 14:29
@HLeithner
Copy link
Member

I'm not against svg I would really see it in joomla. I personally don't want that the cms requires JS in frontend for so basic things like showing an image.

@infograf768
Copy link
Member

My problem with this solution is that it is not adapted to all the languages we have (in 3.x) as it does not take into account special cases.
I am not also against svg by principle.
For the flags which by usage are always displayed at their real size, we don't really need them though.

@dgrammatiko
Copy link
Contributor Author

@infograf768 that is a problem that designers should take care. I tried to do the heavy lifting here

always displayed in their actual size

Right, so what happens if you zoom in...
There’s no one size since the web moved to responsive design, you should be aware of that, it’s a trend formore than 8 years

@Bakual
Copy link
Contributor

Bakual commented Aug 5, 2018

I don't have an issue with SVGs as well. This PR however is bad for various things. As a starter it doesn't solve a single thing but tries instead to do multiple stuff. As you see in the comments, the questions come from various points. One thing is changing a simple dropdown to a webcomponent, another thing is changing gifs to svgs. And last but not least it broke several other places where those gifs were also used.

Using SVGs for those flags is a very old topic. You already suggested it in #12014. It doesn't seem to be such an important topic given said PR is still open (for almost two years now).

@dgrammatiko
Copy link
Contributor Author

dgrammatiko commented Aug 5, 2018

One thing is changing a simple dropdown to a webcomponent

First and foremost this will be changed to a web component one way or another because J4 will not ship with bbootstrap.js in the core. But I didn't touch the dropdown so this is false...

And last but not least it broke several other places where those gifs were also used.

This is an RFC. It wasn't meant to be a complete solution...

@brianteeman
Copy link
Contributor

  1. If performance is a reason to change then you need to prove that
  2. How will we handle languages that are not associated to a country and therefore dont have a flag

@Fedik
Copy link
Member

Fedik commented Aug 5, 2018

How will we handle languages that are not associated to a country and therefore dont have a flag

off topic: use flag of Earth https://en.wikipedia.org/wiki/File:Flag_of_Earth.svg 😃

@brianteeman
Copy link
Contributor

The only sensible option from my perspective is to stop trying to use a country flag to represent language and drop the images all together

@HLeithner
Copy link
Member

First and foremost this will be changed to a web component one way or another because J4 will not ship with bbootstrap.js in the core. But I didn't touch the dropdown so this is false...

you only changed the frontend and there is no bootstrap if I check this correctly

don't we have flags in the backend too?

The only sensible option from my perspective is to stop trying to use a country flag to represent language and drop the images all together

And the flags are ok because we have flags for each language flavor

@Bakual
Copy link
Contributor

Bakual commented Aug 5, 2018

The only sensible option from my perspective is to stop trying to use a country flag to represent language and drop the images all together

Actually, I think that should be left to the site integrator to decide. As long as it is optional, all is fine for me.
But I have no clue why there should be a JavaScript (webcompoent) involved for anything related to this.

@mbabker
Copy link
Contributor

mbabker commented Aug 5, 2018

But I have no clue why there should be a JavaScript (webcompoent) involved for anything related to this.

As soon as it becomes anything more complex than a simple <select> list (if you're using a dropdown variant), that's when it should be. Otherwise, you've got the situation now where it's hard coupled to Bootstrap's Dropdown component. Or in the case of the issue tracker we used Semantic UI for that, which has its own framework specific coupling. Basically, it's the Chosen problem in a different way.

@mbabker
Copy link
Contributor

mbabker commented Aug 5, 2018

The only sensible option from my perspective is to stop trying to use a country flag to represent language and drop the images all together

And the flags are ok because we have flags for each language flavor

But it is still semantically incorrect to align a flag, which is generally a representation of a nation or state, to a language or locale code. That is the issue with the way the images are currently defined.

@ghost
Copy link

ghost commented Aug 6, 2018

Actually, I think that should be left to the site integrator to decide. As long as it is optional, all is fine for me.

Module "Language Switcher" should display language names as default.

@infograf768
Copy link
Member

But it is still semantically incorrect to align a flag, which is generally a representation of a nation or state, to a language or locale code. That is the issue with the way the images are currently defined.

"generally" is the key word here.
Let's forget the word "language flags" and call them "language icons" .
Let's the site administrators decide to use them or not. I do not think that the CMS should take the decision for them.

It is already possible, in the switcher as well as in back-end by defining no flag for the Content. Language.

@Bakual
Copy link
Contributor

Bakual commented Aug 6, 2018

As soon as it becomes anything more complex than a simple list (if you're using a dropdown variant), that's when it should be. Otherwise, you've got the situation now where it's hard coupled to Bootstrap's Dropdown component. Mod_Language doesn't use a Bootstrap Dropdown. It's a regular (chosen) select. Module "Language Switcher" should display language names as default. Should be a simple PR to change that for new installations.

@mbabker
Copy link
Contributor

mbabker commented Aug 6, 2018

As soon as it becomes anything more complex than a simple list (if you're using a dropdown variant), that's when it should be. Otherwise, you've got the situation now where it's hard coupled to Bootstrap's Dropdown component.

Mod_Language doesn't use a Bootstrap Dropdown. It's a regular (chosen) select.

Either way, as soon as a native HTML element requires JavaScript enhancement (be it Bootstrap Dropdown, Semantic UI Dropdown, Chosen, or one of the other 42975 packages that do that), it should be made a custom element in the context of a CMS' environment (no, if I ever built a language switcher capability into my Symfony or Laravel apps would I make it a custom element because I probably wouldn't design something that calls for JavaScript enhancement in the first place and even if that were the case I'd simply be re-using assets that already exist, performance and critical rendering path be damned). Otherwise you are left with vendor lock-in in that particular layout with the tool forcibly loading the assets of whatever enhancer is in use, and whatever dependencies come with it.

@mbabker
Copy link
Contributor

mbabker commented Aug 6, 2018

Let's forget the word "language flags" and call them "language icons" .
Let's the site administrators decide to use them or not. I do not think that the CMS should take the decision for them.

The main point is that languages do not officially have a flag associated with them. So no matter how you phrase it, semantically it is not 100% correct to associate any language with any flag. It works for the majority of cases because a vast majority of languages have origins that align with a particular country or region.

And yes, we as a project can be opinionated as to whether out-of-the-box we will support display of flags as representations of languages. Nothing stops anyone who wants that type of display from creating it for their sites, but nothing says we have to support it ourselves either. Although once something has landed in Joomla it takes a miracle to remove it, so I guess we're stuck now, even if there were a majority consensus that this doesn't need to be a core feature.

@brianteeman
Copy link
Contributor

Some things can be changed and using a country symbol to indicate a language is not only incorrect semantically but creates all sorts of issues that we have to invent solutions for.

If using text etc is good enough for the UN, BBC, Facebook, Twitter etc then it should be good enough for joomla core.

@Bakual
Copy link
Contributor

Bakual commented Aug 6, 2018

@brianteeman Just wondering: If it's completely optional and disabled by default, do you still have an issue with it? Because I think that could be done easily.

@brianteeman
Copy link
Contributor

that is better than the current status quo but i still say it is wrong. As a swiss citizen you should understand

@Bakual
Copy link
Contributor

Bakual commented Aug 6, 2018

As a Swiss, I know the Swiss flag can't be used as a language flag since there is no "Swiss" language. And the funny "flag" we created for de-CH is an even worse idea.
However I see it sometimes that sites or applications use the flags of Germany, France and Italy to refer to the respective language (or language regions) and personally I don't have an issue with that. Everybody here understands clearly the intention in this use case and nobody is offended here.
However I also understand that it doesn't work for many cases. So making it optional seems to be the good swiss compromise 😄

@infograf768
Copy link
Member

infograf768 commented Aug 7, 2018

It is easy to modify our sql to not set an image for en-GB.
Installing a language would also have to be modified to not set an icon automatically.
For Content Languages, we would also make the image field displayed by a showon if another field is set to Yes. That other field (Use Languages Icons) would be set to No by default.

@dgrammatiko
I understand the zoom matter and I welcome the .svg, as long as it is NOT automatically based on the country code of the language tag but still chosen for each Content Language. The svg therefore should be all present in build/ and not fetched from an external site.
What I mean is that we could take off the gif and add all the available country flags svg in the media/mod_languages/image/ folder with the appropriate display code in the switcher which would keep the proportion of the flags and also used (when desired) in back-end too if the Content Language is set to use an image.
Code to modify would be in the layout as well as in mod_languages/tmpl/default.php
HTMLHelper::_('image', 'mod_languages/' . $item->language_image . '.gif', '', null, true) . '&nbsp;' . htmlspecialchars($item->language_title, ENT_COMPAT, 'UTF-8');

As I wrote above, we will need to make specific generic svg icons for Esperanto or Tamil for example.
Obviously all this would not be B/C but as most of 4.0 is not, no p.

@dgrammatiko
Copy link
Contributor Author

The svg therefore should be all present in build/ and not fetched from an external site.

That was my intention, this was just an RFC, there is a comment in the code denoting exactly that, but I didn't want to spend much time here, just showcase the proof of concept.
Can it be done without js, of course, but we're not lazyloading...

Obviously all this would not be B/C but as most of 4.0 is not

Well, this can be done in such a way that we can retain B/C

The point is what other mentioned above and I totally agree: don't use icons AT ALL for the languages!

@SharkyKZ
Copy link
Contributor

SharkyKZ commented Aug 7, 2018

Is it really a good idea to force lazyloading without an option to disable it? Should be done through a plugin, IMO.

@dgrammatiko
Copy link
Contributor Author

@SharkyKZ lazyloading is the defacto best practice for anyone that builds modern websites. I see no reason why someone will not want to lazy load any asset...

@simbus82
Copy link
Contributor

simbus82 commented Sep 24, 2018

Gif in Language switch is an ugly thing from the past.

For example we have in our site this override of mod_languages:

<?php foreach ($list as $language) : ?>
	    <?php if ($language->active) : ?>
    				<span class="flag-lang" >
    					<?php if (file_exists('media/mod_languages/images/'.$language->image.'.svg')) {	
    						echo JHtml::_('image', 'mod_languages/' . $language->image . '.svg', '', null, true);
    					}
						else{
							echo JHtml::_('image', 'mod_languages/' . $language->image . '.gif', '', null, true);
						}
    					?>
    					
    					</span>
	    <?php endif; ?>
<?php endforeach; ?>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request for Comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants