Adds localization features #240

sdellis · 2024-11-13T20:26:46Z

still working out how to pass the locale parameter to every liveview page

github-actions · 2024-11-13T20:28:55Z

Container Scanning Status: ✅ Success


ghcr.io/pulibrary/dpul-collections:pr-240 (debian 12.6)
=======================================================
Total: 0 (HIGH: 0, CRITICAL: 0)

hackartisan

This looks great!!! I left a few comments.

Also I'd love to look together, I don't understand why we have set_locale_hook as well as locale_plug and how they interact. I'm hoping you can show me.

hackartisan · 2024-11-21T19:39:47Z

lib/dpul_collections_web/components/lux-components.ex

+      <lux-menu-bar
+        type="main-menu"
+        v-bind:menu-items={"[
+        {name: '#{gettext("Language")}', component: 'Language', children: [
          {name: 'English', component: 'English', href: '?locale=en'},
          {name: 'Español', component: 'Español', href: '?locale=es'},
          {name: 'Português do Brasil', component: 'Português do Brasil', href: '?locale=pt-BR'}


Should we take out the Brazilian until we get the translations in place?

I think it's fine to leave it. It would be fairly easy to add the Brasilian translations if we wanted to make sure that option works.

hackartisan · 2024-11-21T19:40:02Z

lib/dpul_collections_web/live/home_live.ex

  alias DpulCollections.Solr
  alias DpulCollectionsWeb.Live.Helpers

+  # on_mount SetLocaleHook


old comment?

hackartisan · 2024-11-21T19:40:48Z

lib/dpul_collections_web/live/home_live.ex

        <p class="text-xl text-center">
-          We invite you to be inspired by our globally diverse collections of <%= @item_count %> Ephemera items. We can't wait to see how you use these materials to support your unique research.
+          <%= gettext("We invite you to be inspired by our globally diverse collections of") %> <%= @item_count %>
+          <%= gettext(


Does the break between tags render as a newline?

It does. The text is very long. I usually have soft wrapping on in my editor, but I try to avoid lines that are that long. I am not tied to having a line break here.

There's a max width style on it so I don't think it will ever get too long on the actual screen.

hackartisan · 2024-11-21T19:46:34Z

priv/gettext/default.pot

+
+#: lib/dpul_collections_web/components/core_components.ex:85
+#, elixir-autogen, elixir-format
+msgid "We can't find the internet"


this one is my favorite

hackartisan · 2024-11-21T19:47:37Z

priv/gettext/es/LC_MESSAGES/default.po

+#: lib/dpul_collections_web/live/search_live.ex:186
+#, elixir-autogen, elixir-format
+msgid "Previous"
+msgstr "anterior"


should this one be capitalized?

hackartisan · 2024-11-21T19:49:10Z

test/dpul_collections_web/live/home_live_test.exs

@@ -6,7 +6,7 @@ defmodule DpulCollectionsWeb.HomeLiveTest do
  test "GET /", %{conn: conn} do
    count = DpulCollections.Solr.document_count()
    conn = get(conn, ~p"/")
-    assert html_response(conn, 200) =~ "#{count} Ephemera items"
+    assert html_response(conn, 200) =~ "#{count}\n      Ephemera items"


I think it would be better if we could not have this newline. Relevant to my earlier comment

hackartisan · 2024-11-21T19:49:18Z

test/dpul_collections_web/set_locale_hook_test.exs

@@ -0,0 +1,58 @@
+defmodule DpulCollectionsWeb.SetLocaleHookTest do
+  use ExUnit.Case, async: true
+  # import Phoenix.LiveView


@sdellis one more

hackartisan · 2024-11-21T20:38:24Z

lib/dpul_collections_web/set_locale_hook.ex

@@ -0,0 +1,13 @@
+defmodule DpulCollectionsWeb.SetLocaleHook do


Could we add a module doc that says something like, "set up locale for front-end use"

hackartisan · 2024-11-21T20:38:47Z

lib/plug/locale_plug.ex

+defmodule DpulCollectionsWeb.LocalePlug do
+  @moduledoc """
+  Fetch locale and set it for the specified Gettext backend.
+


maybe add something like, "sets it in the session so the front end can use it"

hackartisan · 2024-11-21T20:41:02Z

priv/gettext/en/LC_MESSAGES/default.po

+#: lib/dpul_collections_web/live/search_live.ex:69
+#, elixir-autogen, elixir-format, fuzzy
+msgid "Search Results"
+msgstr "Search"


Remove this one

hackartisan · 2024-11-21T20:41:20Z

priv/gettext/en/LC_MESSAGES/default.po

+#: lib/dpul_collections_web/live/search_live.ex:75
+#, elixir-autogen, elixir-format
+msgid "Search"
+msgstr "Search"


…parameter to every liveview page

…lves

…anguage header and put the locale in the session

…ings

hackartisan

🎉

sdellis changed the title ~~WIP: adds localization features~~ Adds localization features Nov 20, 2024

sdellis marked this pull request as ready for review November 20, 2024 22:19

hackartisan requested changes Nov 21, 2024

View reviewed changes

hackartisan reviewed Nov 21, 2024

View reviewed changes

sdellis added 10 commits November 21, 2024 16:50

Adds localization features; still working out how to pass the locale …

b620ecd

…parameter to every liveview page

uses gettext on every liveview mount so we don't have to repeat ourse…

ae2bfd7

…lves

uses a router plug to set the locale from params > cookies > accept-l…

1f8ca5b

…anguage header and put the locale in the session

fixes existing tests

7c2a03b

remove the passing of the locale as a param in home_live

a282c81

adds tests for localization hook and plug; also cleans up test warnings

deb4df8

adds documentation and Spanish translations

fc6014c

translates home page wording and update documentation for editing str…

fcbff5f

…ings

fixes failing test due to newly wrapped line of expected output

a859ffd

addresses code review change requests

3f2bfa1

sdellis force-pushed the i123-localization branch from 7a37b52 to 3f2bfa1 Compare November 21, 2024 21:51

removes unnecessary comment

5ca336d

hackartisan approved these changes Nov 22, 2024

View reviewed changes

hackartisan merged commit f157ac1 into main Nov 22, 2024
4 checks passed

hackartisan deleted the i123-localization branch November 22, 2024 15:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds localization features #240

Adds localization features #240

sdellis commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading

hackartisan left a comment

hackartisan Nov 21, 2024

sdellis Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

sdellis Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan Nov 21, 2024

hackartisan left a comment

		@@ -0,0 +1,13 @@
		defmodule DpulCollectionsWeb.SetLocaleHook do

Adds localization features #240

Adds localization features #240

Conversation

sdellis commented Nov 13, 2024 • edited Loading

github-actions bot commented Nov 13, 2024 • edited Loading

Container Scanning Status: ✅ Success

hackartisan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hackartisan left a comment

Choose a reason for hiding this comment

sdellis commented Nov 13, 2024 •

edited

Loading

github-actions bot commented Nov 13, 2024 •

edited

Loading