Skip to content

Reimplement localization of page labels - #12

Merged
JohnThomson merged 1 commit into
BloomBooks:masterfrom
andrew-polk:BL251_PageLabelsL10N
Oct 10, 2014
Merged

Reimplement localization of page labels#12
JohnThomson merged 1 commit into
BloomBooks:masterfrom
andrew-polk:BL251_PageLabelsL10N

Conversation

@andrew-polk

Copy link
Copy Markdown
Contributor

No description provided.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like you're taking out the hard-coded localizations for 2.0...shouldn't the comment go too?

Or maybe I'm misunderstanding...the new code does much the same except not if there's already a data-i18n. Filling in the original text as a key does not feel to me like "adding hard-coded localizations" and we shouldn't be putting code that is only for 2.0 into our default branch, so something is wrong with this comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I didn't really understand the comment. I thought maybe it meant that we are hard-coding it for 2.0 but would like to change that for the future. But I agree this isn't really hard-coded. I will remove the comment.

@JohnThomson

Copy link
Copy Markdown
Contributor

Seems like what this change actually does is to protect explicit localization keys from the hack we do to provide them when missing. I don't see any actual localization (or even new internationalization).

@andrew-polk

Copy link
Copy Markdown
Contributor Author

This code was commented out because it had two problems.

  1. The text inside the div was originally being used as the key so after switching from English to language X, one could not switch the language again.
    This must have been fixed with other changes to the localization code because simply uncommenting the code made it work okay.

  2. The translated text was getting added to the tmx file.
    I fixed this by only adding the localization key when we first add the data-i18n attribute.

I'll change the comment to reflect we are reimplementing localization for labels.

@andrew-polk andrew-polk changed the title Localize page labels Reimplement localization of page labels Oct 8, 2014
@andrew-polk
andrew-polk force-pushed the BL251_PageLabelsL10N branch from 022f202 to c0ca062 Compare October 8, 2014 12:24
JohnThomson added a commit that referenced this pull request Oct 10, 2014
Reimplement localization of page labels
@JohnThomson
JohnThomson merged commit 2d2ff41 into BloomBooks:master Oct 10, 2014
@andrew-polk
andrew-polk deleted the BL251_PageLabelsL10N branch October 14, 2014 17:33
hatton added a commit that referenced this pull request Jun 26, 2026
#7949)

Code changes responding to the PR review:

- Team Collections (review #1/#3/#4/#13): BloomBridge's mutating external
  endpoints (add/update/process-book) now fail fast on a Team Collection rather
  than silently corrupting a shared collection (they don't honor checkout, the
  TeamCollection.status file, or TC renames). Adds
  CollectionModel.IsEditableCollectionATeamCollection and
  ExternalApi.RefuseIfTeamCollection, and documents the non-TC assumption on
  CollectionModel.AddBookFromFolder.

- Off-screen JS bridge cleanup (#6/#7/#8) in BookProcessor: the capture trigger
  is now fire-and-forget (its result was discarded), the redundant
  Application.DoEvents() in the poll loop is removed (the sync call already pumps
  the loop), and WaitForJavascriptResult documents why the off-screen processor
  keeps the poll-a-window-global approach rather than the live editor's async
  API callback.

- Async page-readiness (#10/#11/#15): SetImageDisplaySizeIfCalledFor now
  registers a requestPageContent delay around its asynchronous image-info fetch
  and resize, so captureContentForExternalProcessing waits for image sizing to
  finish before capturing. Corrects the editablePage.ts comments that wrongly
  called the load-time fix-ups "synchronous", and clarifies the natural-size
  fallback in autoFitImageOverTextSplits.

- autoFit overlays (#14): fitOneImageOverTextSplit now also skips pages whose top
  pane has non-text overlays (canvas elements other than the background image),
  not just text overlays.

- Smaller comment fixes: clarify the "already in the collection" guard in
  AddBookFromFolder (#2); document that the process-book overlay message is
  intentionally not localized (#12); note that priorForeground is captured once
  on purpose (#5); move the JSON-vs-delimiter note next to the code it describes
  (#16); raise the ExternalBusyOverlay z-index (#17).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants