UI review bug fixes

app/javascript/controllers/submit_on_change_controller.js

@@ -0,0 +1,7 @@
+import { Controller } from "@hotwired/stimulus"
+
+export default class extends Controller {
+  submit() {
+    this.element.requestSubmit()
+  }
+}

app/views/accounts/index.html.erb

@@ -20,16 +20,7 @@
   <article id="<%= kind %>-accounts">
     <h2 id="<%= kind %>-accounts-heading"><%= table[:heading] %></h2>
 
-    <% if table[:page].number > 1 || table[:page].before_last? %>
-      <nav class="pagination">
-        <% if table[:page].number > 1 %>
-          <%= link_to t("pagination.previous"), url_for(request.query_parameters.merge(table[:page_param] => table[:page].number - 1)), rel: "prev" %>
-        <% end %>
-        <% if table[:page].before_last? %>
-          <%= link_to t("pagination.next"), url_for(request.query_parameters.merge(table[:page_param] => table[:page].next_param)), rel: "next" %>
-        <% end %>
-      </nav>
-    <% end %>
+    <%= render "shared/pagination", page: table[:page], param: table[:page_param] %>
 
     <table class="accounts" id="<%= kind %>-accounts-table" data-stack="medium" role="table"
            aria-labelledby="<%= kind %>-accounts-heading">
@@ -55,15 +46,6 @@
       </tbody>
     </table>
 
-    <% if table[:page].number > 1 || table[:page].before_last? %>
-      <nav class="pagination">
-        <% if table[:page].number > 1 %>
-          <%= link_to t("pagination.previous"), url_for(request.query_parameters.merge(table[:page_param] => table[:page].number - 1)), rel: "prev" %>
-        <% end %>
-        <% if table[:page].before_last? %>
-          <%= link_to t("pagination.next"), url_for(request.query_parameters.merge(table[:page_param] => table[:page].next_param)), rel: "next" %>
-        <% end %>
-      </nav>
-    <% end %>
+    <%= render "shared/pagination", page: table[:page], param: table[:page_param] %>
   </article>
 <% end %>

app/views/layouts/admin.html.erb

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <html
+  lang="<%= I18n.locale %>"
   data-controller="theme"
   data-color-scheme="<%= theme_attributes[:"data-color-scheme"] %>"
   data-light-theme="<%= theme_attributes[:"data-light-theme"] %>"

app/views/layouts/application.html.erb

@@ -1,5 +1,6 @@
 <!DOCTYPE html>
 <html
+  lang="<%= I18n.locale %>"
   data-controller="theme"
   data-color-scheme="<%= theme_attributes[:"data-color-scheme"] %>"
   data-light-theme="<%= theme_attributes[:"data-light-theme"] %>"

app/views/layouts/login.html.erb

@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<%= tag.html(lang: I18n.locale, **theme_attributes, "data-controller": "theme") do %>
+  <head>
+    <title><%= Appkit.config.app_display_name %></title>
+    <meta name="viewport" content="width=device-width,initial-scale=1">
+    <meta name="color-scheme" content="light dark">
+    <meta name="view-transition" content="same-origin">
+    <%= csrf_meta_tags %>
+    <%= csp_meta_tag %>
+    <%= render "appkit/shared/pwa_meta" %>
+
+    <%= stylesheet_link_tag "application", media: "all", "data-turbo-track": "reload" %>
+    <%= javascript_importmap_tags %>
+  </head>
+  <body data-appkit-login>
+    <main>
+      <%= render "appkit/shared/flashes" %>
+      <%= yield %>
+    </main>
+  </body>
+<% end %>

app/views/shared/_pagination.html.erb

@@ -1,14 +1,14 @@
-<%# locals: (page:) %>
+<%# locals: (page:, param: :page) %>
 <% show_previous = page.number > 1 %>
 <% show_next = page.before_last? %>
 
 <% if show_previous || show_next %>
   <nav class="pagination">
     <% if show_previous %>
-      <%= link_to t("pagination.previous"), url_for(request.query_parameters.merge(page: page.number - 1)), rel: "prev" %>
+      <%= link_to t("pagination.previous"), url_for(request.query_parameters.merge(param => page.number - 1)), rel: "prev" %>
     <% end %>
     <% if show_next %>
-      <%= link_to t("pagination.next"), url_for(request.query_parameters.merge(page: page.next_param)), rel: "next" %>
+      <%= link_to t("pagination.next"), url_for(request.query_parameters.merge(param => page.next_param)), rel: "next" %>
     <% end %>
   </nav>
 <% end %>

app/views/users/_user.html.erb

@@ -2,10 +2,10 @@
 <div>
   <%= link_to user.name, user.current? ? edit_user_profile_path(user) : user_profile_path(user) %>
 
-  <%= form_with model: user, url: user_path(user), data: { controller: "form" }, method: :patch do |form| %>
+  <%= form_with model: user, url: user_path(user), data: { controller: "submit-on-change" }, method: :patch do |form| %>
     <label for="<%= dom_id(user, :role) %>">
       <%= t('.role_label', name: user.name, role: user.administrator? ? t('.role_administrator') : t('.role_member')) %>
-      <%= form.check_box :role, { data: { action: "form#submit" }, hidden: true, id: dom_id(user, :role),
+      <%= form.check_box :role, { data: { action: "change->submit-on-change#submit" }, hidden: true, id: dom_id(user, :role),
             disabled: ("disabled" if user.current? || !Current.user.can_administer?) },
             "administrator", "member" %>
     </label>

app/views/users/profiles/edit.html.erb

@@ -1,7 +1,5 @@
 <% content_for(:title) { t(".title") } %>
-<%= render layout: "shared/title_bar", locals: { title: t('.title') } do %>
-  <%= link_to t("common.back"), users_path %>
-<% end %>
+<%= render "shared/title_bar", title: t(".title") %>
 
 <article>
   <%= form_with model: @user, url: user_profile_path(@user) do |form| %>

config/locales/en.yml

@@ -110,8 +110,10 @@ en:
         amount: Amount
         category: Category
       category:
+        direction: Direction
         kind: Kind
         name: Name
+        parent_category: Parent category
       chattel:
         brand: Brand
         kind: Kind
@@ -121,6 +123,7 @@ en:
         notes: Notes
         purchase_price: Purchase price
         purchase_transaction: Transaction
+        purchase_transaction_id: Purchase transaction
         purchased_at: Purchase Date
         serial_number: Serial Number
         warranty_document: Warranty document
@@ -140,10 +143,21 @@ en:
         account: Account
         amount: Amount
         appropriation: Appropriation
+        booked_at: Booked at
         category: Category
+        category_id: Category
+        creditor: Creditor
         date: Date
+        debitor: Debitor
         description: Description
+        id: Id
+        interest_at: Interest at
+        note: Note
+        original_balance_after_mutation: Original balance after mutation
+        original_note: Original note
+        original_tag: Original tag
         proof_of_purchase: Proof of purchase
+        type: Type
       transaction_split:
         appropriation: Appropriation
     errors:

config/locales/it.yml

@@ -110,8 +110,10 @@ it:
         amount: Importo
         category: Categoria
       category:
+        direction: Direzione
         kind: Tipo
         name: Nome
+        parent_category: Categoria principale
       chattel:
         brand: Marca
         kind: Tipo
@@ -121,6 +123,7 @@ it:
         notes: Note
         purchase_price: Prezzo d'acquisto
         purchase_transaction: Transazione
+        purchase_transaction_id: Transazione d'acquisto
         purchased_at: Data d'acquisto
         serial_number: Numero seriale
         warranty_document: Documento di garanzia
@@ -140,10 +143,21 @@ it:
         account: Conto
         amount: Importo
         appropriation: Stanziamento
+        booked_at: Data registrazione
         category: Categoria
+        category_id: Categoria
+        creditor: Creditore
         date: Data
+        debitor: Debitore
         description: Descrizione
+        id: ID
+        interest_at: Data valuta
+        note: Nota
+        original_balance_after_mutation: Saldo originale dopo il movimento
+        original_note: Descrizione originale
+        original_tag: Etichetta originale
         proof_of_purchase: Prova d'acquisto
+        type: Tipo
       transaction_split:
         appropriation: Stanziamento
     errors:

config/locales/nl.yml

@@ -110,8 +110,10 @@ nl:
         amount: Bedrag
         category: Categorie
       category:
+        direction: Richting
         kind: Type
         name: Naam
+        parent_category: Hoofdcategorie
       chattel:
         brand: Merk
         kind: Type
@@ -121,6 +123,7 @@ nl:
         notes: Notities
         purchase_price: Aankoopprijs
         purchase_transaction: Transactie
+        purchase_transaction_id: Aankooptransactie
         purchased_at: Aankoopdatum
         serial_number: Serienummer
         warranty_document: Garantiedocument
@@ -140,10 +143,21 @@ nl:
         account: Rekening
         amount: Bedrag
         appropriation: Begrotingspost
+        booked_at: Boekingsdatum
         category: Categorie
+        category_id: Categorie
+        creditor: Crediteur
         date: Datum
+        debitor: Debiteur
         description: Omschrijving
+        id: ID
+        interest_at: Rentedatum
+        note: Opmerking
+        original_balance_after_mutation: Oorspronkelijk saldo na mutatie
+        original_note: Oorspronkelijke omschrijving
+        original_tag: Oorspronkelijk label
         proof_of_purchase: Aankoopbewijs
+        type: Type
       transaction_split:
         appropriation: Begrotingspost
     errors:

test/integration/form_labels_test.rb

@@ -0,0 +1,58 @@
+require "test_helper"
+
+class FormLabelsTest < ActionDispatch::IntegrationTest
+  LABELS = {
+    nl: {
+      transaction: [ "ID", "Debiteur", "Crediteur", "Categorie", "Opmerking", "Boekingsdatum", "Rentedatum", "Type",
+                     "Oorspronkelijke omschrijving", "Oorspronkelijk saldo na mutatie", "Oorspronkelijk label" ],
+      category: [ "Richting", "Hoofdcategorie" ],
+      chattel: [ "Aankooptransactie" ]
+    },
+    it: {
+      transaction: [ "ID", "Debitore", "Creditore", "Categoria", "Nota", "Data registrazione", "Data valuta", "Tipo",
+                     "Descrizione originale", "Saldo originale dopo il movimento", "Etichetta originale" ],
+      category: [ "Direzione", "Categoria principale" ],
+      chattel: [ "Transazione d'acquisto" ]
+    }
+  }.freeze
+
+  PAGES = {
+    transaction: -> { edit_transaction_path(transactions(:debit_grocery)) },
+    category: -> { edit_category_path(categories(:groceries)) },
+    chattel: -> { edit_chattel_path(chattels(:one)) }
+  }.freeze
+
+  setup do
+    @user = users(:admin)
+  end
+
+  LABELS.each_key do |locale|
+    names = { nl: "Dutch", it: "Italian" }.fetch(locale)
+
+    test "the transaction form labels its fields in #{names}" do
+      assert_labels locale, :transaction
+    end
+
+    test "the category form labels direction and parent category in #{names}" do
+      assert_labels locale, :category
+    end
+
+    test "the chattel form labels the purchase transaction in #{names}" do
+      assert_labels locale, :chattel
+    end
+  end
+
+  private
+
+  def assert_labels(locale, form)
+    @user.update!(locale: locale)
+    sign_in_as(@user)
+
+    get instance_exec(&PAGES.fetch(form))
+
+    assert_response :success
+    LABELS.dig(locale, form).each do |text|
+      assert_select "form label", text: text
+    end
+  end
+end

test/integration/login_layout_test.rb

@@ -0,0 +1,11 @@
+require "test_helper"
+
+class LoginLayoutTest < ActionDispatch::IntegrationTest
+  test "a signed-out visitor's page leaves the theme to the browser" do
+    get new_session_path
+
+    assert_select "html[data-controller=?]", "theme"
+    assert_select "html[data-theme]", count: 0
+    assert_select "html[data-color-scheme]", count: 0
+  end
+end

test/integration/page_language_test.rb

@@ -0,0 +1,33 @@
+require "test_helper"
+
+class PageLanguageTest < ActionDispatch::IntegrationTest
+  test "a Dutch user gets lang nl" do
+    sign_in_as(users(:admin))
+
+    get dashboard_index_path
+
+    assert_select "html[lang=?]", "nl"
+  end
+
+  test "an English user gets lang en" do
+    sign_in_as(users(:member))
+
+    get dashboard_index_path
+
+    assert_select "html[lang=?]", "en"
+  end
+
+  test "the admin layout declares the user's language" do
+    sign_in_as(users(:admin))
+
+    get admin_root_path
+
+    assert_select "html[lang=?]", "nl"
+  end
+
+  test "a signed-out visitor gets the default locale" do
+    get new_session_path
+
+    assert_select "html[lang=?]", I18n.default_locale.to_s
+  end
+end

test/integration/pagination_navigation_test.rb

@@ -21,6 +21,29 @@ class PaginationNavigationTest < ActionDispatch::IntegrationTest
     assert_select "nav.pagination a[rel='next']", 0
   end
 
+  test "external accounts pagination writes external_page" do
+    create_external_accounts(51)
+
+    get accounts_path
+
+    assert_response :success
+    assert_select "nav.pagination a[rel='next'][href*='external_page=2']", 2
+    assert_select "nav.pagination a[rel='next'][href*='own_page']", 0
+
+    get accounts_path, params: { external_page: 2 }
+
+    assert_select "nav.pagination a[rel='prev'][href*='external_page=1']", 2
+    assert_select "nav.pagination a[rel='next']", 0
+  end
+
+  test "accounts pagination keeps the other query parameters" do
+    create_accounts(51)
+
+    get accounts_path, params: { external_page: 1, q: "kept" }
+
+    assert_select "nav.pagination a[rel='next'][href*='own_page=2'][href*='external_page=1'][href*='q=kept']", 2
+  end
+
   test "transactions pagination keeps filter params and does not show next on overflow pages" do
     create_uncategorized_transactions(21)
 
@@ -64,6 +87,15 @@ def create_accounts(count)
       end
     end
 
+    def create_external_accounts(count)
+      count.times do |index|
+        Account.create!(
+          account_numbers: [ AccountNumber.new(number: format("NL98SHOP%010d", index)) ],
+          name: "External Pagination Account #{index}"
+        )
+      end
+    end
+
     def create_uncategorized_transactions(count)
       debitor = accounts(:checking)
       creditor = accounts(:unknown)