From 04a01f1ad8372c1a3063f83572f0981a713bd965 Mon Sep 17 00:00:00 2001 From: Hatton Date: Mon, 9 Dec 2024 17:24:16 -0700 Subject: [PATCH] Monday review with JT --- .storybook/main.ts | 2 +- README.md | 28 ++++++------ package.json | 2 +- src/bloom-player-controls.tsx | 12 +++--- src/bloom-player-core.tsx | 69 +++++++++++------------------- src/navigation.test.ts | 10 +++++ src/navigation.ts | 41 +++++++++--------- src/stories/BloomPlayerIframe.tsx | 2 +- src/stories/multiBook.stories.tsx | 2 +- src/stories/navigation.stories.tsx | 2 +- 10 files changed, 82 insertions(+), 88 deletions(-) diff --git a/.storybook/main.ts b/.storybook/main.ts index aed5006..ce4000f 100644 --- a/.storybook/main.ts +++ b/.storybook/main.ts @@ -8,7 +8,7 @@ const proxy = viteConfigFn({ const config: StorybookConfig = { stories: ["../src/**/*.stories.@(js|jsx|mjs|ts|tsx)"], // For stories that include an iframe and thus need access to bloomplayer.htm and the bundles it loads. - // When using those stories, use `yarn buildForStorybook` to update non-storybook code. + // When using those stories, use `yarn watchForStorybook` to update non-storybook code. staticDirs: ["../dist"], addons: [ "@storybook/addon-links", diff --git a/README.md b/README.md index cfbba00..f93a35d 100644 --- a/README.md +++ b/README.md @@ -4,11 +4,11 @@ This project, _bloom-player_, lets users view and interact with [Bloom](https:// The Bloom project uses _bloom-player_ in the following places: -- In [Bloom Editor](https://github.com/bloombooks/bloomdesktop) / Publish / Bloom Reader tab, for previewing what the book will look like on a device. -- In the [Bloom Reader](https://github.com/bloombooks/bloomreader) Android app -- On [BloomLibrary.org](https://bloomlibrary.org) -- In [BloomPUB Viewer](https://github.com/bloombooks/bloompub-viewer) -- In Android and IOS apps created with Reading App Builder +- In [Bloom Editor](https://github.com/bloombooks/bloomdesktop) / Publish / Bloom Reader tab, for previewing what the book will look like on a device. +- In the [Bloom Reader](https://github.com/bloombooks/bloomreader) Android app +- On [BloomLibrary.org](https://bloomlibrary.org) +- In [BloomPUB Viewer](https://github.com/bloombooks/bloompub-viewer) +- In Android and IOS apps created with Reading App Builder # Using bloom-player in a website @@ -20,7 +20,7 @@ So to embed a book in your website, you just need an iframe element that points ```html ``` @@ -32,7 +32,7 @@ You can customize some aspects of the bloom-player to fit your context. For exam ```html ``` @@ -75,7 +75,7 @@ Default: none (The initial language will be the vernacular language when the boo #### hideFullScreenButton -If true, the Full Screen icon button will not appear in the upper right hand corner of the window. Otherwise, a Full Screen icon button is displayed and allows the user to toggle between full screen and window mode. +If true, the Full Screen icon button will not appear in the upper right hand corner of the window. Otherwise, a Full Screen icon button is displayed and allows the user to toggle between full screen and window mode. Example: `hideFullScreenButton=true` @@ -83,7 +83,7 @@ Default: `false` #### independent -If true, bloom-player reports its own analytics directly to segment. If false, bloom-player sends messages to its host, and the host is responsible for reporting analytics. +If true, bloom-player reports its own analytics directly to segment. If false, bloom-player sends messages to its host, and the host is responsible for reporting analytics. Example: `independent=false` @@ -91,7 +91,7 @@ Default: `true` #### host -If set, provides a host value for analytics. If not set, bloom-player attempts to derive a value from the available information if independent is `true` (information can be limited in an iframe) and does nothing otherwise. +If set, provides a host value for analytics. If not set, bloom-player attempts to derive a value from the available information if independent is `true` (information can be limited in an iframe) and does nothing otherwise. Examples: `host=bloomlibrary` or `host=bloomreader` or `host=bloompubviewer` or `host=readerapp` @@ -106,6 +106,7 @@ Some books are designed to be published together as a collection. These books ma Instead of asking your host to handle the navigation, history, etc., this navigation happens within the one instance of Bloom Player. However, Bloom Player needs your help, because it does not have a way to know what the URL to each book is in the context of your host, which could be a website, an app, a desktop program, etc. So if you want to support these collections of linked books, your host has to do some extra work. Bloom books link to each other using each book's "Instance ID". This is a guid that is given as a property in the `meta.json` file: + ```json { "bookInstanceId":"0b82aab3-61fb-429e-864f-ce7671a3d372", @@ -115,14 +116,15 @@ Bloom books link to each other using each book's "Instance ID". This is a guid t "pageCount":16, ``` -When the user clicks on a link to another book, Bloom Player will first request a resource from `/book/THE-INSTANCE-ID/index.htm`. This will be followed by requests for things like `/book/THE-INSTANCE-ID/basePage.css` and `/book/THE-INSTANCE-ID/image1.png`. +When the user clicks on a link to another book, Bloom Player will first request a resource from `/book/THE-INSTANCE-ID/index.htm`. If you do a redirect to `C:/mybook/index.htm`, then this will be followed by requests for things like `C:/mybook/basePage.css` and `C:/mybook/image1.png`. To support this, your host needs to: + 1. intercept that request 2. extract the Instance ID 3. figure out the path to that book in your system 4. if the target is still zipped in a BloomPUB, you may need to unzip it or read the resource directly out of the archive -5. return the requested file (if your host is running the server) or redirect to a new url (if it is not). +5. redirect to a new url If these steps are slow, you might need to cache or even pre-prepare an index for use at runtime. @@ -149,6 +151,7 @@ Both `yarn storybook` and `yarn dev` do this for you. ### Testing with a book hosted by Bloom To test Bloom Player on a book in the Bloom Editor, follow these steps: + 1. Go to the Publish tab in Bloom, choose "BloomPUB", and click "Preview". 2. Back in this repository, run storybook (`yarn storybook`). 3. Choose the "Live From Bloom Editor" story. @@ -158,6 +161,7 @@ To test Bloom Player on a book in the Bloom Editor, follow these steps: To run unit tests use `yarn test`. This will run all `*.test.ts`, which should be collocated with the thing being tested. ### More info + For more information, see README-advanced.md ## License diff --git a/package.json b/package.json index 45909b7..e8dfe3f 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "prepare": "husky install && git config blame.ignoreRevsFile .git-blame-ignore-revs", "storybook": "storybook dev -p 6006", "// run this when working with stories that use an iframe and bloomplayer.htm instead of the raw react components": " ", - "buildForStorybook": "yarn vite build --watch --mode development", + "watchForStorybook": "yarn vite build --watch --mode development", "strings:download": "crowdin download --config src/l10n/crowdin.yml && yarn strings:compile", "strings:upload": "crowdin upload --config src/l10n/crowdin.yml", "strings:compile": "yarn tsx ./src/l10n/compileL10Strings.ts", diff --git a/src/bloom-player-controls.tsx b/src/bloom-player-controls.tsx index 8a83b27..2101755 100644 --- a/src/bloom-player-controls.tsx +++ b/src/bloom-player-controls.tsx @@ -581,9 +581,7 @@ export const BloomPlayerControls: React.FunctionComponent = ( const amountToMoveDown = (winHeight - actualPageHeight - controlsHeight) / 2; // don't count controlsHeight in what we move down if (amountToMoveDown > 0) { - translateString = `translate(0, ${amountToMoveDown.toFixed( - 0, - )}px) `; + translateString = `translate(0, ${amountToMoveDown.toFixed(0)}px) `; // console.log(`** translating down ${amountToMoveDown}px`); // console.log(` winHeight ${winHeight}px`); // console.log(` desiredPageHeight ${desiredPageHeight}px`); @@ -915,8 +913,8 @@ export const BloomPlayerControls: React.FunctionComponent = ( // we have a page or book jump on our bloom player history stack return BackButtonState.showArrow; else if (showBackButton) { - // so our internal history is empty, but the container is - // wants us to offer a "back" to it. Bloom Reader uses this. + // so our internal history is empty, but the container + // wants us to offer a way "back" to it. Bloom Reader uses this. if (window === window.top) return BackButtonState.showArrow; // Used by bloomlibrary.org if you went straight to a book via some URL @@ -932,7 +930,9 @@ export const BloomPlayerControls: React.FunctionComponent = ( playLabel={playLabel} preferredLanguages={preferredUiLanguages} backClicked={() => { - if (!coreRef.current?.HandleBackButtonClicked()) { + if ( + !coreRef.current?.HandleBackButtonClickedIfHavePlayerHistory() + ) { sendBackToHost(); } }} diff --git a/src/bloom-player-core.tsx b/src/bloom-player-core.tsx index 3084d69..1e1c122 100644 --- a/src/bloom-player-core.tsx +++ b/src/bloom-player-core.tsx @@ -286,7 +286,7 @@ export class BloomPlayerCore extends React.Component { const parsedUrl = new URL(props.url, window.location.origin); this.state.bookUrl = parsedUrl.pathname; if (parsedUrl.hash) { - this.state.startPageId = parsedUrl.hash.substring(1); + this.state.startPageId = parsedUrl.hash.substring(1); // strip the "#" } else { // Copy into state because in a multi-book scenario, the prop.startPageIndex will // always be the initial value, but we don't want to just got to that page of @@ -355,6 +355,22 @@ export class BloomPlayerCore extends React.Component { this.handleDocumentLevelKeyDown(e), ); + // Prevent unwanted behavior on link clicks that accidentally move a bit. + document.addEventListener( + "pointerdown", + (event) => { + if ( + (event.target as HTMLElement).closest("[href], [data-href]") + ) { + // Stop the swiper from starting a drag + event.stopPropagation(); + // Stop the browser from showing a thing like you're trying to drag the link to some other window + event.preventDefault(); + } + }, + { capture: true }, // Let us see this before children see it. + ); + // Clicking on any element that has a data-href attribute will be treated as a link. // This is the simplest way to handle such links that may be scattered on different // types of elements throughout the book. See BL-13879. @@ -411,7 +427,7 @@ export class BloomPlayerCore extends React.Component { return canGoBack(); } // called by bloom-player-controls - public HandleBackButtonClicked(): boolean { + public HandleBackButtonClickedIfHavePlayerHistory(): boolean { const backLocation = goBackInHistoryIfPossible( this.bookInfo.bookInstanceId, ); @@ -543,11 +559,10 @@ export class BloomPlayerCore extends React.Component { // parse the url const u = new URL(newSourceUrl, window.location.origin); - // TODO: the URL parse always introduces a leading slash if it's missing, but currently that breaks... something. - // I feel like I'm hacking around something I should understand better. + // The URL parsing can introduce a slash we don't want when the input URL is not relative. this.sourceUrl = newSourceUrl.startsWith("/") - ? u.pathname - : u.pathname.substring(1); + ? u.pathname // if the original was relative, fine + : u.pathname.substring(1); // if the original was not relative, don't make it look relative // We support a two ways of interpreting URLs. // If the url ends in .htm, it is assumed to be the URL of the htm file that @@ -612,7 +627,7 @@ export class BloomPlayerCore extends React.Component { }, ); const htmlPromise = axios.get(urlOfBookHtmlFile); - console.log("urlOfBookHtmlFile", urlOfBookHtmlFile); + // console.log("urlOfBookHtmlFile", urlOfBookHtmlFile); const metadataPromise = axios.get(this.fullUrl("meta.json")); Promise.all([htmlPromise, metadataPromise, distributionPromise]) .then((result) => { @@ -894,7 +909,7 @@ export class BloomPlayerCore extends React.Component { ); }; - // TODO: most of this should be moved into bookInfo, including the pageIdToIndex map. + // ENHANCE: most of this should be moved into bookInfo, including the pageIdToIndex map. // clear the pageIdToIndex map pageIdToIndex.length = 0; pageIdToIndex["cover"] = 0; @@ -2235,12 +2250,7 @@ export class BloomPlayerCore extends React.Component { .hasAppearanceSystem ? "appearance-system" : "" - } ${ - this.state - .importedBodyAttributes[ - "class" - ] ?? "" - }`} + } ${this.state.importedBodyAttributes["class"] ?? ""}`} // Note: the contents of `slide` are what was in the .htm originally. // So you would expect that this would replace any changes made to the dom by the activity or other code. // You would expect that we would lose the answers a user made in an activity. @@ -2361,37 +2371,6 @@ export class BloomPlayerCore extends React.Component { // Other internal links won't work and are disabled; external ones are forced to // open a new tab as the results of trying to display a web page in the bloom-player // iframe or webview can be confusing. - // private fixInternalHyperlinks(div: HTMLDivElement | null): void { - // if (!div) { - // return; - // } - // const anchors = Array.from(div.getElementsByTagName("a") ?? []); - // anchors.forEach((a) => { - // const href = a.getAttribute("href"); // not a.href, which has the full page address prepended. - // if (href?.startsWith("#")) { - // const pageId = href.substring(1); - // const pageNum = this.state.pageIdToIndexMap[pageId]; - // if (pageNum !== undefined) { - // a.href = ""; // may not be needed, on its own was unsuccessful in stopping attempted default navigation - // a.onclick = (ev) => { - // ev.preventDefault(); // don't try to follow the link, other than by the slideTo below - // ev.stopPropagation(); // don't allow parent listeners, particularly the one that toggles the nav bar - // this.swiperInstance.slideTo(pageNum); - // }; - // } else { - // // no other kind of internal link makes sense, so let them be ignored. - // a.onclick = (ev) => { - // ev.preventDefault(); - // ev.stopPropagation(); - // }; - // } - // } else { - // // an external link. It will likely confuse things to follow it inside whatever iframe or webview - // // bloom player may be running in, so encourage opening a new tab or similar action. - // a.setAttribute("target", "blank"); - // } - // }); - // } // What we need to do when the page narration is completed (if autoadvance, go to next page). public handlePageNarrationComplete = (page: HTMLElement | undefined) => { diff --git a/src/navigation.test.ts b/src/navigation.test.ts index c996e2a..64cf2bc 100644 --- a/src/navigation.test.ts +++ b/src/navigation.test.ts @@ -132,6 +132,16 @@ describe("Navigation functions", () => { const thirdBack = goBackInHistoryIfPossible("book1"); expect(thirdBack).toBeUndefined(); }); + + test("doesn't die if an href is empty", () => { + const event = createClickEvent(""); + const result = checkClickForBookOrPageJump( + event, + "doesn't matter", + () => "doesn't matter", + ); + expect(result).toEqual(undefined); + }); }); }); diff --git a/src/navigation.ts b/src/navigation.ts index 37f2318..78aaa29 100644 --- a/src/navigation.ts +++ b/src/navigation.ts @@ -5,15 +5,15 @@ type ILocation = { bookUrl?: string; pageId?: string }; export function canGoBack() { return jumpHistory.length > 0; } -export function goBackInHistoryIfPossible( +export function tryPopPlayerHistory( currentBookId: string, ): ILocation | undefined { const previousLocation = jumpHistory.pop(); if (previousLocation) { - console.log( - "Going Back. Current History is: ", - JSON.stringify(jumpHistory, null, 2), - ); + // console.log( + // "Going Back. Current History is: ", + // JSON.stringify(jumpHistory, null, 2), + // ); return { bookUrl: currentBookId === previousLocation.bookId @@ -24,15 +24,12 @@ export function goBackInHistoryIfPossible( } return undefined; } -function urlFromBookId(bookId: string) { - return `/book/${bookId}/index.htm`; -} export function checkClickForBookOrPageJump( event: any, currentBookInstanceId: string, - getCurentPageId: () => string, -): ILocation { + getCurrentPageId: () => string, +): ILocation | undefined { const linkElement = (event.target as HTMLElement).closest( "[href], [data-href]", ); @@ -41,10 +38,9 @@ export function checkClickForBookOrPageJump( event.stopPropagation(); const href: string = - (linkElement.attributes["href"] && - linkElement.attributes["href"].nodeValue) || - (linkElement.attributes["data-href"] && - linkElement.attributes["data-href"].nodeValue); + (linkElement.getAttribute("href") || + linkElement.getAttribute("data-href")) ?? + ""; if (href.startsWith("http://") || href.startsWith("https://")) { // This is a generic external link. We open it in a new window or tab. @@ -66,7 +62,7 @@ export function checkClickForBookOrPageJump( : urlFromBookId(previousLocation.bookId!), pageId: previousLocation.pageId, }; - } + } else return undefined; } else if (href.startsWith("/book/")) { const target = parseTargetBookUrl(href); targetBookId = target.bookId; @@ -93,16 +89,21 @@ export function checkClickForBookOrPageJump( return { pageId: targetPageId }; } - return {}; // nothing to do as far as navigation goes + return undefined; // nothing to do as far as navigation goes } function pushLocation(bookId: string, pageId: string, comment: string) { jumpHistory.push({ bookId, pageId }); - console.log( - comment + " Current History is: ", - JSON.stringify(jumpHistory, null, 2), - ); + // console.log( + // comment + " Current History is: ", + // JSON.stringify(jumpHistory, null, 2) + // ); } + +function urlFromBookId(bookId: string) { + return `/book/${bookId}/index.htm`; +} + function parseTargetBookUrl(url: string) { // the format is "/book/BOOKID#PAGEID" where the page id is optional const bloomUrl = new URL(url, window.location.origin); diff --git a/src/stories/BloomPlayerIframe.tsx b/src/stories/BloomPlayerIframe.tsx index c85c487..5342d0f 100644 --- a/src/stories/BloomPlayerIframe.tsx +++ b/src/stories/BloomPlayerIframe.tsx @@ -6,7 +6,7 @@ export function BloomPlayerIframe({ bookUrl, bookPageIndex, showBackButton }) { if (!useBuildIsReady()) { return (
- Waiting for Bloom Player to build. See "yarn buildForStorybook" + Waiting for Bloom Player to build. See "yarn watchForStorybook"
); } diff --git a/src/stories/multiBook.stories.tsx b/src/stories/multiBook.stories.tsx index d3135aa..17c44a0 100644 --- a/src/stories/multiBook.stories.tsx +++ b/src/stories/multiBook.stories.tsx @@ -3,7 +3,7 @@ import type { Meta, StoryObj } from "@storybook/react"; import { BloomPlayerIframe } from "./BloomPlayerIframe"; /* This uses an iframe, so normal dev server compilation is not effective. -Therefore, when developing with this, use `yarn buildForStorybook` to update non-storybook code. +Therefore, when developing with this, use `yarn watchForStorybook` to update non-storybook code. The point of this story is really just to host a book that has links to a second book. What makes that possible is the proxy defined in .storybook/main.ts, not here. diff --git a/src/stories/navigation.stories.tsx b/src/stories/navigation.stories.tsx index 841ba6c..ca5fd1c 100644 --- a/src/stories/navigation.stories.tsx +++ b/src/stories/navigation.stories.tsx @@ -3,7 +3,7 @@ import { BloomPlayerIframe } from "./BloomPlayerIframe"; import { BloomPlayerTester } from "./BloomPlayerTester"; /* This uses an iframe, so normal dev server compilation is not effective. -Therefore, when developing with this, use `yarn buildForStorybook` to update non-storybook code. +Therefore, when developing with this, use `yarn watchForStorybook` to update non-storybook code. The point of this story is really just to host a book that has links to a second book. What makes that possible is the proxy defined in .storybook/main.ts, not here.