-
-
Notifications
You must be signed in to change notification settings - Fork 400
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
fix: enable all languages on world; fix language dropdown (#9209) #9225
fix: enable all languages on world; fix language dropdown (#9209) #9225
Conversation
Kudos, SonarCloud Quality Gate passed! |
Codecov Report
@@ Coverage Diff @@
## main #9225 +/- ##
==========================================
+ Coverage 47.80% 47.82% +0.01%
==========================================
Files 65 65
Lines 20292 20292
Branches 4932 4932
==========================================
+ Hits 9701 9705 +4
+ Misses 9336 9333 -3
+ Partials 1255 1254 -1
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Hi @NooaLumi thank you so much for this contribution 🎉 Before we merge let's raise two questions:
|
Hum the test that fails seems to be exactly on this ! |
It works but for some reason I get a different behavior on Firefox and Chrome. On Firefox I get the click to open behaviour, while on Chrome I have the old hover to open behaviour. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really appreciate that you try to tackle this, but let's had a hardcoded exception for world in code instead off adding all langages in taxonomy.
@@ -265,6 +265,7 @@ zh_min_nan:Sè-kài | |||
zh_yue:世界 | |||
country_code_2:en:world | |||
wikidata:en:Q16502 | |||
language_codes:en:so,af,ga,gn,ay,az,id,ms,bi,nb,bs,ca,sn,ny,cy,da,de,na,et,en,es,eu,fj,ch,mg,mg,fr,gv,sm,prs,gd,ho,hr,rw,rn,xh,zu,it,pl,mh,kl,sw,ht,lv,to,lt,ro,la,gl,lb,hu,mt,nl,nd,uz,pt,qu,mi,st,tn,ss,sk,sl,nr,fi,sv,sg,vi,ve,tk,tr,tl,ts,ku,is,cs,el,be,bg,tg,ky,mo,mk,mn,ru,sr,uk,kk,hy,he,ur,ar,fa,ps,dv,ne,hi,bn,ta,si,th,lo,dz,my,ka,ti,am,km,ja,zh,ko |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative would be to add an exception in the code, but why not that way, it does not move too much, so it might be ok indeed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the test is failing because of this, because we use this property to add the no_index flag (and this is very important).
See Display.pm on line 773
if ((!($lc ~~ $country_languages{$cc})) or $subdomain =~ /^(ssl-)?api/) {
# Use robots.txt with disallow: / for all agents
$request_ref->{deny_all_robots_txt} = 1;
if ($request_ref->{is_crawl_bot} eq 1) {
$request_ref->{no_index} = 1;
}
}
So instead of adding this property, I propose that you add an exception in the code you modified in Display.pm with an hardcoded exception for "world".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the no_index is very important because otherwise we get hammered by indexing bots.
@alexgarel yes indeed, we forbid indexation on all world pages that are not in English language. |
What
Screenshot
Related issue(s) and discussion
List of language codes mapped to their names, in alphabetical order (for reference):