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

Chore (i18n): Make punctuation usage consistent in translations #9610

Merged
merged 13 commits into from Jun 13, 2023
5 changes: 4 additions & 1 deletion docs/dev-practices.md
Expand Up @@ -72,11 +72,14 @@ queries and rules. Our queries are linted through clj-kondo and
[datalog-parser](https://github.com/lambdaforge/datalog-parser). clj-kondo will
error if it detects an invalid query.

### Invalid translations
### Translations

Our translations can be configured incorrectly. We can catch some of these
mistakes [as noted here](./contributing-to-translations.md#fix-mistakes).

Punctuation and delimiting characters (e.g. `:`, `:`, `?`) should be part of the translatable string.
Those characters and their position may vary depending on the language.

### Spell Checker

We use [typos](https://github.com/crate-ci/typos) to spell check our source code.
Expand Down
16 changes: 8 additions & 8 deletions e2e-tests/util/search-modal.ts
Expand Up @@ -15,28 +15,28 @@ export async function createRandomPage(page: Page) {
// Fill [placeholder="Search or create page"]
await page.fill('[placeholder="Search or create page"]', randomTitle)
// Click text=/.*New page: "new page".*/
await page.click('text=/.*New page: ".*/')
await page.click('text=/.*New page:".*/')
// Wait for h1 to be from our new page
await page.waitForSelector(`h1 >> text="${randomTitle}"`, { state: 'visible' })
// wait for textarea of first block
await page.waitForSelector('textarea >> nth=0', { state: 'visible' })

return randomTitle;
}

export async function createPage(page: Page, page_name: string) {// Click #search-button
await closeSearchBox(page)
await page.click('#search-button')
// Fill [placeholder="Search or create page"]
await page.fill('[placeholder="Search or create page"]', page_name)
// Click text=/.*New page: "new page".*/
await page.click('text=/.*New page: ".*/')
await page.click('text=/.*New page:".*/')
// wait for textarea of first block
await page.waitForSelector('textarea >> nth=0', { state: 'visible' })

return page_name;
}

export async function searchAndJumpToPage(page: Page, pageTitle: string) {
await closeSearchBox(page)
await page.click('#search-button')
Expand All @@ -50,7 +50,7 @@ export async function searchAndJumpToPage(page: Page, pageTitle: string) {
/**
* type a search query into the search box
* stop at the point where search box shows up
*
*
* @param page the pw page object
* @param query the search query to type into the search box
* @returns the HTML element for the search results ui
Expand All @@ -61,6 +61,6 @@ export async function searchPage(page: Page, query: string): Promise<ElementHand
await page.waitForSelector('[placeholder="Search or create page"]')
await page.type('[placeholder="Search or create page"]', query, { delay: 10 })
await page.waitForTimeout(2000) // wait longer for search contents to render

return page.$$('#ui__ac-inner>div');
}
2 changes: 1 addition & 1 deletion e2e-tests/whiteboards.spec.ts
Expand Up @@ -293,7 +293,7 @@ test('create a block', async ({ page }) => {

test('expand the block', async ({ page }) => {
await page.keyboard.press('Escape')
await page.click('.logseq-tldraw .tl-context-bar .tie-object-expanded ')
await page.keyboard.press(modKey + '+ArrowDown')
await page.waitForTimeout(100)

await expect(page.locator('.logseq-tldraw .tl-logseq-portal-container .tl-logseq-portal-header')).toHaveCount(1)
Expand Down
13 changes: 3 additions & 10 deletions src/main/electron/listener.cljs
Expand Up @@ -5,7 +5,6 @@
[datascript.core :as d]
[dommy.core :as dom]
[electron.ipc :as ipc]
[frontend.context.i18n :refer [t]]
[frontend.db :as db]
[frontend.db.model :as db-model]
[frontend.fs.sync :as sync]
Expand Down Expand Up @@ -35,9 +34,7 @@
[]
;; only persist current db!
;; TODO rename the function and event to persist-db
(repo-handler/persist-db! {:before #(notification/show!
(ui/loading (t :graph/persist))
:warning)
(repo-handler/persist-db! {:before #(ui/notify-graph-persist!)
:on-success #(ipc/ipc "persistent-dbs-saved")
:on-error #(ipc/ipc "persistent-dbs-error")}))

Expand Down Expand Up @@ -139,15 +136,11 @@
;; fire back "broadcastPersistGraphDone" on done
(fn [data]
(let [repo (bean/->clj data)
before-f #(notification/show!
(ui/loading (t :graph/persist))
:warning)
before-f #(ui/notify-graph-persist!)
after-f #(ipc/ipc "broadcastPersistGraphDone")
error-f (fn []
(after-f)
(notification/show!
(t :graph/persist-error)
:error))
(ui/notify-graph-persist-error!))
handlers {:before before-f
:on-success after-f
:on-error error-f}]
Expand Down
12 changes: 6 additions & 6 deletions src/main/frontend/components/assets.cljs
Expand Up @@ -88,11 +88,11 @@
:disabled (string/blank? val)
:on-click on-submit)]]))

(rum/defc restart-button [active?]
(when active?
(ui/button (t :plugin/restart)
:on-click #(js/logseq.api.relaunch)
:small? true :intent "logseq")))
(rum/defc restart-button
[]
(ui/button (t :plugin/restart)
:on-click #(js/logseq.api.relaunch)
:small? true :intent "logseq"))

(rum/defcs ^:large-vars/data-var alias-directories
< rum/reactive
Expand Down Expand Up @@ -215,7 +215,7 @@
#(state/set-assets-alias-enabled! (not alias-enabled?))
true)]
[:span
(restart-button alias-enabled-changed?)]]
(when alias-enabled-changed? (restart-button))]]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


(when alias-enabled?
[:div.pt-4
Expand Down
11 changes: 6 additions & 5 deletions src/main/frontend/components/block.cljs
Expand Up @@ -1946,11 +1946,12 @@
(not= "nil" marker))
{:class (str (string/lower-case marker))})
(when bg-color
{:style {:background-color (if (some #{bg-color} ui/block-background-colors)
(str "var(--ls-highlight-color-" bg-color ")")
bg-color)
:color (when-not (some #{bg-color} ui/block-background-colors) "white")}
:class "px-1 with-bg-color"}))
(let [built-in-color? (ui/built-in-color? bg-color)]
{:style {:background-color (if built-in-color?
(str "var(--ls-highlight-color-" bg-color ")")
bg-color)
:color (when-not built-in-color? "white")}
:class "px-1 with-bg-color"})))

;; children
(let [area? (= :area (keyword (:hl-type properties)))
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/components/container.cljs
Expand Up @@ -541,7 +541,7 @@
db-restoring?
[:div.mt-20
[:div.ls-center
(ui/loading (t :loading))]]
(ui/loading)]]

:else
[:div
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/components/conversion.cljs
Expand Up @@ -149,7 +149,7 @@
[:p (t :file-rn/need-action)])
[:p
(ui/button
(str (t :file-rn/all-action) " (" (count rename-items) ")")
(t :file-rn/all-action (count rename-items))
:on-click <rename-all
:class "text-md p-2 mr-1")
(t :file-rn/or-select-actions)
Expand Down
10 changes: 6 additions & 4 deletions src/main/frontend/components/editor.cljs
Expand Up @@ -131,7 +131,7 @@
nil

(empty? matched-pages)
(cons (str (t :new-page) ": " q) matched-pages)
(cons q matched-pages)

;; reorder, shortest and starts-with first.
:else
Expand All @@ -142,8 +142,8 @@
matched-pages)]
(if (gstring/caseInsensitiveStartsWith (first matched-pages) q)
(cons (first matched-pages)
(cons (str (t :new-page) ": " q) (rest matched-pages)))
(cons (str (t :new-page) ": " q) matched-pages))))]
(cons q (rest matched-pages)))
(cons q matched-pages))))]
(ui/auto-complete
matched-pages
{:on-chosen (page-handler/on-chosen-handler input id q pos format)
Expand All @@ -154,7 +154,9 @@
{:children
[:div.flex
(when (db-model/whiteboard-page? page-name) [:span.mr-1 (ui/icon "whiteboard" {:extension? true})])
(search/highlight-exact-query page-name q)]
[:div.flex.space-x-1
[:div (when-not (db/page-exists? page-name) (t :new-page))]
(search/highlight-exact-query page-name q)]]
:open? chosen?
:manual? true
:fixed-position? true
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/components/file.cljs
Expand Up @@ -152,7 +152,7 @@
;; wait for content load
(and format
(contains? (gp-config/text-formats) format))
(ui/loading "Loading ...")
logseq-cldwalker marked this conversation as resolved.
Show resolved Hide resolved
(ui/loading)

:else
[:div (t :file/format-not-supported (name format))])]))
Expand Down
4 changes: 2 additions & 2 deletions src/main/frontend/components/file_sync.cljs
Expand Up @@ -598,7 +598,7 @@

[:div.version-list
(if loading?
[:div.p-4 (ui/loading "Loading...")]
[:div.p-4 (ui/loading)]
(for [version version-files]
(let [version-uuid (get-version-key version)
local? (some? (:relative-path version))]
Expand Down Expand Up @@ -704,7 +704,7 @@

;; ready loading
[:div.flex.items-center.h-full.justify-center.w-full.absolute.ready-loading
(ui/loading "Loading...")]]))
(ui/loading)]]))

(defn pick-page-histories-panel [graph-uuid page-name]
(fn []
Expand Down
2 changes: 1 addition & 1 deletion src/main/frontend/components/header.cljs
Expand Up @@ -134,7 +134,7 @@
:icon (ui/icon "bulb")})

(when (and (state/sub :auth/id-token) (user-handler/logged-in?))
{:title (str (t :logout) " (" (user-handler/email) ")")
{:title (t :logout-user (user-handler/email))
:options {:on-click #(user-handler/logout)}
:icon (ui/icon "logout")})]
(concat page-menu-and-hr)
Expand Down
8 changes: 4 additions & 4 deletions src/main/frontend/components/page.cljs
Expand Up @@ -819,7 +819,7 @@
[:div.mt-3.text-center.sm:mt-0.sm:ml-4.sm:text-left
[:h3#modal-headline.text-lg.leading-6.font-medium
(if orphaned-pages?
(str (t :remove-orphaned-pages) "?")
(t :remove-orphaned-pages)
(t :page/delete-confirmation))]]]

[:table.table-auto.cp__all_pages_table.mt-4
Expand Down Expand Up @@ -855,7 +855,7 @@
(close-fn)
(doseq [page-name (map :block/name pages)]
(page-handler/delete! page-name #()))
(notification/show! (str (t :tips/all-done) "!") :success)
(notification/show! (t :tips/all-done) :success)
(js/setTimeout #(refresh-fn) 200)))]]))

(rum/defc pagination
Expand Down Expand Up @@ -1040,15 +1040,15 @@
[:div.r.flex.items-center.justify-between
[:div
(ui/tippy
{:html [:small (str (t :page/show-whiteboards) " ?")]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

? ? 😆

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to bring in the old comments. See #8722 (comment)
Punctuation should be part of the translation (also see #8722 (comment) that didn't make it into this PR). The question mark is superfluous in this case, so removing it is a better option in my opinion, instead of invalidating the translations to include it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with this change. Just thought the original was funny.

Punctuation should be part of the translation

Agree with '?' and '!' but what about delimiting characters like ':' or '-'? Moving those characters into translation will likely make it easier to forget to delimit in some languages. Grep for '(str (t' for examples. Whatever we decide we should document in the translation guide and if possible remove counter examples

Copy link
Collaborator Author

@sprocketc sprocketc Jun 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgetting to delimit could happen, but having the ability to properly translate the app should be our top priority in my opinion. RTL languages is also something that we need to take into account. I think @sawhney17 implemented a plugin, but I didn't have the chance to test it yet.

we should document in the translation guide and if possible remove counter examples

💯

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgetting to delimit could happen, but having the ability to properly translate the app should be the top priority in my opinion

Seems reasonable. Could you remove some of the counter examples from the above grep and document the punctation guidance?

Copy link
Collaborator Author

@sprocketc sprocketc Jun 12, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replaced all (str (t instances and invalidated the previous translations . We still have to fix some remaining cases (grep " (t ). When all of these are fixed, it might be a good idea to update our linter. I added a note to dev-practices.md for now.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. 👍

{:html [:small (t :page/show-whiteboards)]
:arrow true}
[:a.button.whiteboard
{:class (util/classnames [{:active (boolean @*whiteboard?)}])
:on-click #(reset! *whiteboard? (not @*whiteboard?))}
(ui/icon "whiteboard" {:extension? true :style {:fontSize ui/icon-size}})])]
[:div
(ui/tippy
{:html [:small (str (t :page/show-journals) " ?")]
{:html [:small (t :page/show-journals)]
:arrow true}
[:a.button.journal
{:class (util/classnames [{:active (boolean @*journal?)}])
Expand Down
8 changes: 4 additions & 4 deletions src/main/frontend/components/plugins.cljs
Expand Up @@ -270,7 +270,7 @@
(if installing-or-updating?
(t :plugin/updating)
(if new-version
(str (t :plugin/update) " 👉 " new-version)
[:span (t :plugin/update) " 👉 " new-version]
(t :plugin/check-update)))]])

(ui/toggle (not disabled?)
Expand Down Expand Up @@ -554,7 +554,7 @@
:options {:on-click #(reset! *sort-by :stars)}
:icon (ui/icon (aim-icon :stars))}

{:title (str (t :plugin/title) " (A - Z)")
{:title (t :plugin/title "A - Z")
:options {:on-click #(reset! *sort-by :letters)}
:icon (ui/icon (aim-icon :letters))}])
{}))
Expand Down Expand Up @@ -586,7 +586,7 @@
:options {:on-click
#(p/let [root (plugin-handler/get-ls-dotdir-root)]
(js/apis.openPath (str root "/preferences.json")))}}
{:title [:span.flex.items-center (ui/icon "bug") (str (t :plugin/open-logseq-dir) "\u00A0") [:code "~/.logseq"]]
{:title [:span.flex.items-center.whitespace-nowrap.space-x-1 (ui/icon "bug") (t :plugin/open-logseq-dir) [:code "~/.logseq"]]
:options {:on-click
#(p/let [root (plugin-handler/get-ls-dotdir-root)]
(js/apis.openPath root))}}]))
Expand Down Expand Up @@ -1192,7 +1192,7 @@
(if check-pending?
(notify!
[:div
[:div (str (t :plugin/checking-for-updates))]
[:div (t :plugin/checking-for-updates)]
(when sub-content [:p.opacity-60 sub-content])]
(ui/loading ""))
(when uid (notification/clear! uid))))
Expand Down
4 changes: 2 additions & 2 deletions src/main/frontend/components/repo.cljs
Expand Up @@ -107,7 +107,7 @@
[:div.pl-1.content.mt-3

[:div
[:h2.text-lg.font-medium.my-4 (str (t :graph/local-graphs) ":")]
[:h2.text-lg.font-medium.my-4 (t :graph/local-graphs)]
(when (seq local-graphs)
(repos-inner local-graphs))

Expand All @@ -123,7 +123,7 @@
[:div
[:hr]
[:div.flex.align-items.justify-between
[:h2.text-lg.font-medium.my-4 (str (t :graph/remote-graphs) ":")]
[:h2.text-lg.font-medium.my-4 (t :graph/remote-graphs)]
[:div
(ui/button
[:span.flex.items-center "Refresh"
Expand Down
4 changes: 2 additions & 2 deletions src/main/frontend/components/right_sidebar.cljs
Expand Up @@ -123,11 +123,11 @@
[(t :right-side-bar/help) (onboarding/help)]

:page-graph
[(str (t :right-side-bar/page-graph))
[(t :right-side-bar/page-graph)
(page/page-graph)]

:history
[(str (t :right-side-bar/history))
[(t :right-side-bar/history)
(history)]

:block-ref
Expand Down
8 changes: 4 additions & 4 deletions src/main/frontend/components/search.cljs
Expand Up @@ -257,8 +257,8 @@
{:name icon
:class "highlight"
:extension? true}
[:div.text.font-bold (str label ": ")
[:span.ml-1 name]]))
[:div.text.font-bold label
[:span.ml-2 name]]))

(defn- search-item-render
[search-q {:keys [type data alias]}]
Expand Down Expand Up @@ -312,7 +312,7 @@

:else
(do (log/error "search result with non-existing uuid: " data)
(str (t :search/cache-outdated)))))])
(t :search/cache-outdated))))])

:page-content
(let [{:block/keys [snippet uuid]} data ;; content here is normalized
Expand All @@ -327,7 +327,7 @@
(if page
(page-content-search-result-item repo uuid format snippet search-q search-mode)
(do (log/error "search result with non-existing uuid: " data)
(str (t :search/cache-outdated)))))]))
(t :search/cache-outdated))))]))

nil)]))

Expand Down
6 changes: 2 additions & 4 deletions src/main/frontend/extensions/excalidraw.cljs
Expand Up @@ -17,8 +17,7 @@
[goog.object :as gobj]
[goog.functions :refer [debounce]]
[rum.core :as rum]
[frontend.mobile.util :as mobile-util]
[frontend.context.i18n :refer [t]]))
[frontend.mobile.util :as mobile-util]))

(def excalidraw (r/adapt-class Excalidraw))

Expand Down Expand Up @@ -148,8 +147,7 @@
(when (:file option)
(cond
db-restoring?
[:div.ls-center
(ui/loading (t :loading))]
[:div.ls-center (ui/loading)]

(false? loading?)
(draw-inner data option)
Expand Down