Depends on how much @gravitystorm is against settings in cookies
https://github.com/openstreetmap/openstreetmap-website/pull/4985#issuecomment-2551576536.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-260485246
Is there anything that should be fixed in this PR?
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-2602117571
You are receiving this because you are subscribed to this thread.
Message ID:
___
> The only downside of the current solution is that there will be only those
languages shown, which have shared.language_selector.language key.
Those that have translations that are maintained, so not much of a downside.
I also had a more defensive version of that key with `language: THIS LANGU
@nertc commented on this pull request.
> @@ -0,0 +1,6 @@
+$(document).ready(function () {
+ $(".language-change-trigger").on("change", function () {
Code was updated according to the recommendation.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openst
This PR was updated according to the suggestion of @AntonKhorev.
As described in the `CONTRIBUTING.md` only updated language config was `en`:
> ## i18n
>
> If you make a change that involve the locale files (in `config/locales`) then
> please only submit changes to the `en.yml` file. The other fi
@nertc pushed 1 commit.
25fd157b9ce6e7945bf5d5e9871d04388e9e5f19 Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/80708ff796458168ebb1060bcb16e07ecf2e6fb5..25fd157b9ce6e7945bf5d5e9871d04388e9e5f19
You are receiving this because you
@AntonKhorev commented on this pull request.
> + <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
+ .select { |locale| locale }
+ .sort_by { |locale| locale[:english_name] }
+ .each do |language| %>
+
@nertc commented on this pull request.
> + <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
+ .select { |locale| locale }
+ .sort_by { |locale| locale[:english_name] }
+ .each do |language| %>
+><%
@AntonKhorev commented on this pull request.
> + <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
+ .select { |locale| locale }
+ .sort_by { |locale| locale[:english_name] }
+ .each do |language| %>
+
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,16 @@
+
+
+
+
+ <% unless disabled %>
+
+ <% Locale.available
+ .map { |locale| Language.find_by(:code => locale.to_s) }
You'll get a ton of db queries:
```
Language Load (0.2ms) SELECT "languages".* F
@nertc pushed 1 commit.
80708ff796458168ebb1060bcb16e07ecf2e6fb5 Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/41cb49e6a084607132ed9818e9139925f924a19b..80708ff796458168ebb1060bcb16e07ecf2e6fb5
You are receiving this because you
@nertc pushed 1 commit.
41cb49e6a084607132ed9818e9139925f924a19b Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/f5754b8cd7e4c60add5abc9b7f7b247a1b77..41cb49e6a084607132ed9818e9139925f924a19b
You are receiving this because you
Looks like the languages you get from `Language` don't quite match the ones
expected by `Locale.list`. For example, we have `pt-BR` and `pt` in `Language`
but `pt` and `pt-PT` in translations. As a result you can't select European
Portuguese.
--
Reply to this email directly or view it on GitHu
@nertc commented on this pull request.
> @@ -0,0 +1,12 @@
+
+
+
+
+ <% unless disabled %>
+
+ <% Language.order(:english_name).each do |language| %>
+><%=
language.name %>
If there was not a cookie, none of the options would be preselected. But for
the better UX this
@nertc commented on this pull request.
> @@ -78,6 +78,15 @@
<%= link_to t("layouts.about"), about_path, :class =>
"dropdown-item" %>
+
+<% if current_user && current_user.id %>
+ <%= link_to(preferences_path, :class => "nav-link text-secondary
@nertc pushed 1 commit.
f5754b8cd7e4c60add5abc9b7f7b247a1b77 Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/34d2e252b3c9fe2735ea607d43caea8b849613c5..f5754b8cd7e4c60add5abc9b7f7b247a1b77
You are receiving this because you
@AntonKhorev commented on this pull request.
> @@ -0,0 +1,12 @@
+
+
+
+
+ <% unless disabled %>
+
+ <% Language.order(:english_name).each do |language| %>
+><%=
language.name %>
Shouldn't the comparison here be with `I18n.locale.to_s` instead of
`request.cookies["_os
@AntonKhorev commented on this pull request.
> @@ -78,6 +78,15 @@
<%= link_to t("layouts.about"), about_path, :class =>
"dropdown-item" %>
+
+<% if current_user && current_user.id %>
+ <%= link_to(preferences_path, :class => "nav-link text-sec
@nertc commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path, :clas
@nertc pushed 1 commit.
34d2e252b3c9fe2735ea607d43caea8b849613c5 Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/713eaaaff248f5b0a91680e081f7217f69fd7c4b..34d2e252b3c9fe2735ea607d43caea8b849613c5
You are receiving this because you
@nertc commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path, :clas
@AntonKhorev commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path,
@nertc commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path, :clas
@AntonKhorev commented on this pull request.
> @@ -76,6 +85,15 @@
<%= link_to t("layouts.copyright"), copyright_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.help"), help_path, :class =>
"dropdown-item" %>
<%= link_to t("layouts.about"), about_path,
PR was updated to include SVG in the HTML. I tried several different methods to
move SVG from sprite, including adding it to the SvgHelper module and adding
separate file for it, but current solution turned out to be the most optimal
one (SvgHelper had conflicts with the linter because of long l
@nertc pushed 1 commit.
713eaaaff248f5b0a91680e081f7217f69fd7c4b Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/e02866a8b5003ae8a8c82c141db9c522b209680b..713eaaaff248f5b0a91680e081f7217f69fd7c4b
You are receiving this because you
@AntonKhorev Okay, thanks. I'll update it to ``. It will be more
comfortable to reuse and will produce cleaner code.
--
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201#issuecomment-2352793179
You are receiving this because you a
> Adding icon to the sprite was solely for the accordance with the current
> state and practices of the project.
Currently we use both `` and css sprite maps. One problem with sprite maps
is that it's more difficult to control the colors of the icons.
app/assets/images/sprite.svg is mainly used
Thank you for the review
@tomhughes I've updated accordingly
@AntonKhorev If it's okay to directly use SVG instead of the `.icon` class
practice of the project, I am all for it. I think it's even better solution and
produces cleaner, more reusable code.
--
Reply to this email directly or view
@nertc pushed 1 commit.
e02866a8b5003ae8a8c82c141db9c522b209680b Add locale selector
--
View it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/5201/files/64c4f7e97c2dcf835e6ed9c1fc1242310def14d9..e02866a8b5003ae8a8c82c141db9c522b209680b
You are receiving this because you
@AntonKhorev Currently, as I found in the project, for icons we use `.icon`
class, which uses sprite with `background-image` and `background-position`
properties. Adding icon to the sprite was solely for the accordance with the
current state and practices of the project.
--
Reply to this email
31 matches
Mail list logo
