From b30f427e024243f1da9c94f63a1077e5f92229e1 Mon Sep 17 00:00:00 2001 From: Lokesh Dhakar Date: Sat, 22 Aug 2026 05:09:24 -0700 Subject: [PATCH 1/5] Put the shelf button on search results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces my_books/dropper with on /search, behind a `use_shelf_button` flag on SearchResultsWork so the macro's five other callers stay on the dropper until each has been looked at on its own. The page is not user-cached, so shelf, rating and check-in state are server-rendered as attributes rather than fetched — but the buttons are stateless by contract, so the page also has to apply what they report. js/my-books/shelf-buttons.js does that, which is also what keeps two buttons for one work in step and what tells the check-in prompt the shelf moved. The prompt itself is untouched: it stays a sibling under the same `#check-in-container-` id the dropper used. get_patrons_reading_states() batches the three lookups the dropper did per result — the old path cost 2N queries for shelf and last-read-date alone. ILE's selection guard ignores clicks inside `a, button, details`. A click in a shadow root retargets to the host, so opening the popover was also selecting the row for the librarian toolbar. Labels come from my_books/book_actions_i18n.html, rendered once per request — every book on a page wants the same strings. Note that removing a book from a shelf deletes its check-ins server-side. The dropper warns first; this deliberately does not. Claude-Session: https://claude.ai/code/session_01XJ9QNXNg3UfbtH3dmDxUfV --- openlibrary/core/bookshelves_events.py | 18 +++ openlibrary/i18n/messages.pot | 132 +++++++++++++---- openlibrary/macros/SearchResultsWork.html | 7 +- .../SelectionManager/SelectionManager.js | 6 +- openlibrary/plugins/openlibrary/js/index.js | 10 ++ .../plugins/openlibrary/js/my-books/index.js | 2 + .../openlibrary/js/my-books/shelf-buttons.js | 79 +++++++++++ openlibrary/plugins/upstream/mybooks.py | 44 +++++- .../tests/test_mybooks_reading_states.py | 77 ++++++++++ .../templates/my_books/book_actions_i18n.html | 52 +++++++ .../templates/my_books/shelf_button.html | 42 ++++++ openlibrary/templates/work_search.html | 7 +- static/css/components/searchResultItemCta.css | 7 + tests/e2e/shelf-button.spec.ts | 134 ++++++++++++++++++ tests/unit/js/shelfButtons.test.js | 134 ++++++++++++++++++ 15 files changed, 721 insertions(+), 30 deletions(-) create mode 100644 openlibrary/plugins/openlibrary/js/my-books/shelf-buttons.js create mode 100644 openlibrary/plugins/upstream/tests/test_mybooks_reading_states.py create mode 100644 openlibrary/templates/my_books/book_actions_i18n.html create mode 100644 openlibrary/templates/my_books/shelf_button.html create mode 100644 tests/e2e/shelf-button.spec.ts create mode 100644 tests/unit/js/shelfButtons.test.js diff --git a/openlibrary/core/bookshelves_events.py b/openlibrary/core/bookshelves_events.py index 50b34257531..22650c61baa 100644 --- a/openlibrary/core/bookshelves_events.py +++ b/openlibrary/core/bookshelves_events.py @@ -66,6 +66,24 @@ def get_latest_event_date(cls, username, work_id, event_type): results = list(oldb.query(query, vars=data)) return results[0] if results else None + @classmethod + def get_latest_event_dates_for_works(cls, username: str, work_ids: list[int], event_type: int) -> dict[int, dict]: + """The most recent event of `event_type` per work, for a batch of works. + + The single-work version costs a query each, which a page rendering + twenty search results cannot afford. + """ + if not work_ids: + return {} + oldb = db.get_db() + data = {"username": username, "work_ids": work_ids, "event_type": event_type} + query = ( + f"SELECT DISTINCT ON (work_id) work_id, id, event_date FROM {cls.TABLENAME}" + " WHERE username=$username AND work_id IN $work_ids AND event_type=$event_type" + " ORDER BY work_id, event_date DESC" + ) + return {row.work_id: row for row in oldb.query(query, vars=data)} + @classmethod def get_user_yearly_read_counts(cls, username: str) -> list[tuple[int, int]]: """Returns books read by year for a given user.""" diff --git a/openlibrary/i18n/messages.pot b/openlibrary/i18n/messages.pot index a4dd81bc4e6..38b3f0edc75 100644 --- a/openlibrary/i18n/messages.pot +++ b/openlibrary/i18n/messages.pot @@ -718,7 +718,7 @@ msgstr "" msgid "Now" msgstr "" -#: admin/graphs.html admin/index.html my_books/check_ins/check_in_form.html my_books/check_ins/check_in_prompt.html trending.html +#: admin/graphs.html admin/index.html my_books/book_actions_i18n.html my_books/check_ins/check_in_form.html my_books/check_ins/check_in_prompt.html trending.html msgid "Today" msgstr "" @@ -751,13 +751,13 @@ msgid "Earliest trending data is from October 2017" msgstr "" #. Label for the reading log shelf for books the user plans to read in the future (bookshelf ID 1). Used as a button label and shelf name. -#: account/mybooks.html account/sidebar.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html +#: account/mybooks.html account/sidebar.html my_books/book_actions_i18n.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html msgid "Want to Read" msgstr "" #. Display name for the "currently-reading" reading log shelf used in page headings and breadcrumbs. #. Label for the reading log shelf for books the user is actively reading (bookshelf ID 2). Used as a button label and shelf name. -#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html +#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/book_actions_i18n.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html msgid "Currently Reading" msgstr "" @@ -768,7 +768,7 @@ msgstr "" #. Display name for the "stopped-reading" reading log shelf used in page headings and breadcrumbs. #. Label for the reading log shelf for books the user stopped reading before finishing (bookshelf ID 4). Used as a button label and shelf name. #. Label for the reading log shelf for books the user stopped reading (bookshelf ID 4). Used as a button label and shelf name. -#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html +#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/book_actions_i18n.html my_books/dropdown_content.html my_books/primary_action.html search/sort_options.html trending.html msgid "Stopped Reading" msgstr "" @@ -1293,7 +1293,7 @@ msgstr "" #. Display name for the "already-read" reading log shelf used in page headings and breadcrumbs. #. Label for the reading log shelf for books the user has finished reading (bookshelf ID 3). Used as a button label and shelf name. -#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/dropdown_content.html my_books/primary_action.html openlibrary/plugins/upstream/mybooks.py search/sort_options.html +#: account/mybooks.html account/readinglog_shelf_name.html account/sidebar.html my_books/book_actions_i18n.html my_books/dropdown_content.html my_books/primary_action.html openlibrary/plugins/upstream/mybooks.py search/sort_options.html msgid "Already Read" msgstr "" @@ -1395,7 +1395,7 @@ msgstr "" msgid "Yes! Please!" msgstr "" -#: EditButtons.html account/notifications.html account/privacy.html admin/block.html admin/spamwords.html covers/manage.html +#: EditButtons.html account/notifications.html account/privacy.html admin/block.html admin/spamwords.html covers/manage.html my_books/book_actions_i18n.html msgid "Save" msgstr "" @@ -1652,7 +1652,7 @@ msgstr "" msgid "See All" msgstr "" -#: account/sidebar.html type/list/edit.html type/series/edit.html +#: account/sidebar.html my_books/book_actions_i18n.html type/list/edit.html type/series/edit.html msgid "Create a list" msgstr "" @@ -3113,7 +3113,7 @@ msgstr "" msgid " by %(name)s" msgstr "" -#: books/daisy.html +#: books/daisy.html my_books/book_actions_i18n.html msgid "Back" msgstr "" @@ -5153,6 +5153,106 @@ msgstr "" msgid "comments" msgstr "" +#: my_books/book_actions_i18n.html +#, python-format +msgid "Actions for %(title)s" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "Save %(title)s to your reading log" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "%(title)s is on your reading log" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "More options for %(title)s" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Rate this book" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "Rate %(rating)s of 5" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Clear rating" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Add to list" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "List name" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Create" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Filter lists…" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "You have no lists yet." +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "No lists match." +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Loading lists…" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "%(count)s items" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "In %(count)s of your lists" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Something went wrong. Please try again." +msgstr "" + +#: my_books/book_actions_i18n.html my_books/check_ins/check_in_prompt.html +msgid "When did you finish this book?" +msgstr "" + +#: my_books/book_actions_i18n.html +#, python-format +msgid "In %(year)s" +msgstr "" + +#: my_books/book_actions_i18n.html +msgid "Other date" +msgstr "" + +#: my_books/book_actions_i18n.html my_books/check_ins/check_in_form.html +msgid "Year" +msgstr "" + +#: my_books/book_actions_i18n.html my_books/check_ins/check_in_form.html +msgid "Month" +msgstr "" + +#: my_books/book_actions_i18n.html my_books/check_ins/check_in_form.html +msgid "Day" +msgstr "" + #: my_books/dropdown_content.html msgid "Remove From Shelf" msgstr "" @@ -5262,26 +5362,14 @@ msgstr "" msgid "Year:" msgstr "" -#: my_books/check_ins/check_in_form.html -msgid "Year" -msgstr "" - #: my_books/check_ins/check_in_form.html msgid "Month:" msgstr "" -#: my_books/check_ins/check_in_form.html -msgid "Month" -msgstr "" - #: my_books/check_ins/check_in_form.html msgid "Day:" msgstr "" -#: my_books/check_ins/check_in_form.html -msgid "Day" -msgstr "" - #: my_books/check_ins/check_in_form.html msgid "Delete Event" msgstr "" @@ -5299,10 +5387,6 @@ msgstr "" msgid "Read " msgstr "" -#: my_books/check_ins/check_in_prompt.html -msgid "When did you finish this book?" -msgstr "" - #: my_books/check_ins/check_in_prompt.html msgid "Other" msgstr "" diff --git a/openlibrary/macros/SearchResultsWork.html b/openlibrary/macros/SearchResultsWork.html index 38b32683f10..a4fb0b85dba 100644 --- a/openlibrary/macros/SearchResultsWork.html +++ b/openlibrary/macros/SearchResultsWork.html @@ -1,4 +1,4 @@ -$def with (doc, decorations=None, cta=True, availability=None, extra=None, attrs=None, rating=None, highlighting=None, show_librarian_extras=False, include_dropper=False, blur=False, footer=None, seq_index=None) +$def with (doc, decorations=None, cta=True, availability=None, extra=None, attrs=None, rating=None, highlighting=None, show_librarian_extras=False, include_dropper=False, blur=False, footer=None, seq_index=None, use_shelf_button=False, reading_state=None, shelf_labels=None, hide_rating=False) $code: max_rendered_authors = 9 @@ -268,7 +268,10 @@

$ edition_key = doc.get('edition_key') and doc.get('edition_key')[0] $if edition_key: $ edition_key = '/books/%s' % edition_key - $:render_template('my_books/dropper', doc, edition_key=edition_key, async_load=True) + $if use_shelf_button: + $:render_template('my_books/shelf_button', doc, edition_key=edition_key, reading_state=reading_state, labels=shelf_labels, hide_rating=hide_rating) + $else: + $:render_template('my_books/dropper', doc, edition_key=edition_key, async_load=True) $if rating: $:rating diff --git a/openlibrary/plugins/openlibrary/js/ile/utils/SelectionManager/SelectionManager.js b/openlibrary/plugins/openlibrary/js/ile/utils/SelectionManager/SelectionManager.js index 63bcf111251..621984fac9c 100644 --- a/openlibrary/plugins/openlibrary/js/ile/utils/SelectionManager/SelectionManager.js +++ b/openlibrary/plugins/openlibrary/js/ile/utils/SelectionManager/SelectionManager.js @@ -84,9 +84,11 @@ export default class SelectionManager { * @param {MouseEvent & { currentTarget: HTMLElement }} clickEvent */ processClick(clickEvent) { - // If there is text selection or the click is on a link that isn't a select handle, don't do anything + // If there is text selection or the click is on a link that isn't a select handle, don't do anything. + // `ol-shelf-button` is named explicitly: a click inside a shadow root + // retargets to the host, so the `)} @@ -671,6 +776,21 @@ export class OlBookActions extends LitElement { `; } + /** + * The end of a shelf row. Already Read carries the date it holds and a + * chevron, because it leads to the date pane; the others only mark the + * shelf the book is on. + */ + _renderShelfTrail(row) { + if (row.id === SHELF.ALREADY_READ) { + return html` + ${this.readDate ? html`${formatReadDate(this.readDate)}` : nothing} + + `; + } + return this.shelf === row.id ? html`` : nothing; + } + _renderStars() { const shown = this._hoverRating || this.rating || 0; // Once rated, the caption becomes an actionable "Clear rating" link. @@ -700,16 +820,32 @@ export class OlBookActions extends LitElement { `; } + /** + * Which row the recorded date is, so the pane shows the answer it already + * holds instead of reading as unanswered. Anything that is neither exactly + * today nor one of the offered years — a partial date included — belongs + * to "Other date". + */ + get _answeredBy() { + if (!this.readDate) return null; + const now = new Date(); + if (this.readDate === partialDate({ year: now.getFullYear(), month: now.getMonth() + 1, day: now.getDate() })) return 'today'; + if (quickYears(now).some(y => this.readDate === String(y))) return this.readDate; + return 'other'; + } + /** * Asked straight after the reader marks a book read. Two one-tap answers - * cover most cases; "Other date" swaps in the selects rather than taking a - * fourth pane, the same way the lists pane swaps in its create form. + * cover most cases; "Other date" discloses the selects underneath itself + * rather than replacing the rows or taking a fourth pane, so the two quick + * answers stay one tap away and the row you pressed stays on screen as the + * anchor. The track measures the pane, so the growth animates for free. * * A year on its own is a valid check-in, which is what makes "In 2026" * offerable at all. */ _renderCheckIn() { - const thisYear = new Date().getFullYear(); + const answered = this._answeredBy; return html`
${this.t('whenFinished')}
- ${this._pickingDate ? this._renderDateFields() : html` -
- - + ${quickYears().map(year => html` + - -
- `} + `)} + + + ${this._pickingDate ? this._renderDateFields() : nothing} `; } @@ -744,19 +904,28 @@ export class OlBookActions extends LitElement { const years = Array.from({ length: 121 }, (_, i) => thisYear - i); const days = month ? new Date(Number(year), Number(month), 0).getDate() : 31; return html` - + { if (e.key === 'Escape') { e.stopPropagation(); this._toggleDatePicker(); } }} + > +
- this._setDatePart('year', e.target.value)}> + + ${years.map(y => html``)} - this._setDatePart('month', e.target.value)}> + + ${MONTHS().map((name, i) => html``)} - this._setDatePart('day', e.target.value)}> + + ${Array.from({ length: days }, (_, i) => i + 1).map(d => html``)}
@@ -842,11 +1011,33 @@ export class OlBookActions extends LitElement { if (changed.has('_pane')) this._syncTrackHeight(); } + connectedCallback() { + super.connectedCallback(); + document.addEventListener('ol-list-created', this._onListCreatedElsewhere); + } + disconnectedCallback() { super.disconnectedCallback(); this._resizeObserver?.disconnect(); + document.removeEventListener('ol-list-created', this._onListCreatedElsewhere); } + /** + * A list created elsewhere on the page — a sibling popover or the legacy + * dropper — folded into this popover's pane so it stays honest without a + * refetch. Legacy creations also drop the shared cache: popovers that have + * not loaded yet must not resolve from a promise that predates the list. + */ + _onListCreatedElsewhere = (e) => { + if (e.target === this) return; + const { key, name, seedKey } = e.detail || {}; + if (!key) return; + if (e.target?.tagName !== 'OL-BOOK-ACTIONS') resetListsCache(); + if (this._lists && !(key in this._lists)) { + this._lists = { [key]: { listName: name, members: seedKey ? [seedKey] : [] }, ...this._lists }; + } + }; + /** Size the track to the active pane so the panel doesn't stretch to the taller one. */ _syncTrackHeight() { const pane = this.shadowRoot.querySelector(`.pane:nth-child(${this._paneIndex + 1})`); @@ -903,6 +1094,12 @@ export class OlBookActions extends LitElement { async _onShelfClick(shelfId) { const previous = this.shelf; + // Already Read leads to the date pane — that is what its chevron says, + // and it is the only way to change a date once given. Coming off the + // shelf is the main button's job. + if (shelfId === SHELF.ALREADY_READ && previous === SHELF.ALREADY_READ) { + return this._openCheckIn(); + } const removing = previous === shelfId; this.shelf = removing ? null : shelfId; this._busy = true; @@ -952,16 +1149,25 @@ export class OlBookActions extends LitElement { async _openCheckIn() { this._pane = 'checkIn'; - this._pickingDate = false; - this._date = { year: '', month: '', day: '' }; + // A date the shortcuts cannot express would otherwise sit unseen + // behind a collapsed row, so the pane opens on it. Focus still lands + // on the first row: the reader is being shown their answer, not asked + // to retype it. + this._pickingDate = this._answeredBy === 'other'; + // Seeded from the date already given, so "Other date" opens on it + // rather than making the reader re-enter what they are amending. + const [year = '', month = '', day = ''] = (this.readDate || '').split('-'); + this._date = { year, month: month.replace(/^0/, ''), day: day.replace(/^0/, '') }; await this.updateComplete; this.shadowRoot.querySelector(`.pane:nth-child(${PANES.indexOf('checkIn') + 1}) .row`)?.focus({ preventScroll: true }); } - async _startPickingDate() { - this._pickingDate = true; + /** Focus follows the disclosure: into the selects, and back to the row on collapse. */ + async _toggleDatePicker() { + this._pickingDate = !this._pickingDate; await this.updateComplete; - this.shadowRoot.querySelector('.select.year')?.focus({ preventScroll: true }); + const target = this._pickingDate ? '.select.year' : '.date-toggle'; + this.shadowRoot.querySelector(target)?.focus({ preventScroll: true }); } /** Clearing a coarser part clears the finer ones, which the selects disable. */ @@ -977,8 +1183,8 @@ export class OlBookActions extends LitElement { return this._saveCheckIn({ year: now.getFullYear(), month: now.getMonth() + 1, day: now.getDate() }); } - _onThisYear() { - return this._saveCheckIn({ year: new Date().getFullYear() }); + _onYear(year) { + return this._saveCheckIn({ year }); } _onSaveDate(e) { @@ -996,8 +1202,13 @@ export class OlBookActions extends LitElement { if (this._dateBusy) return; this._dateBusy = true; try { - await setCheckIn(this.book.key, { ...date, editionKey: this.book.editionKey }); + const saved = await setCheckIn(this.book.key, { ...date, editionKey: this.book.editionKey, eventId: this.eventId }); trackEvent('CheckInPrompt', date.day ? 'SetDateDay' : date.month ? 'SetDateMonth' : 'SetDateYear'); + this.dispatchEvent(new CustomEvent('ol-book-check-in', { + bubbles: true, + composed: true, + detail: { key: this.book.key, date: partialDate(date), eventId: saved?.id ?? this.eventId ?? null }, + })); this._backToMain(); } catch (error) { this._fail(error); @@ -1082,11 +1293,16 @@ export class OlBookActions extends LitElement { try { const created = await createList(this.userKey, name, this._seedKey); trackEvent('Lists', 'CreateList'); - // Prepend so the new list is visible immediately; the shared cache - // is the same object, so sibling popovers see it too. + // Prepend so the new list is visible immediately. Sibling popovers + // and the legacy dropper hear about it through `ol-list-created`. this._lists = { [created.key]: { listName: name, members: [this._seedKey] }, ...this._lists }; _listsPromise = Promise.resolve(this._lists); this._creating = false; + this.dispatchEvent(new CustomEvent('ol-list-created', { + bubbles: true, + composed: true, + detail: { key: created.key, name, seedKey: this._seedKey }, + })); } catch (error) { this._fail(error); } finally { diff --git a/openlibrary/components/lit/OlOptionsPopover.js b/openlibrary/components/lit/OlOptionsPopover.js index 8c9a5e8e7b0..5f9dbb1e352 100644 --- a/openlibrary/components/lit/OlOptionsPopover.js +++ b/openlibrary/components/lit/OlOptionsPopover.js @@ -149,7 +149,7 @@ export class OlOptionsPopover extends FormAssociatedMixin(LitElement) { width: 16px; height: 16px; margin: 2px 0 0; - accent-color: var(--primary-blue); + accent-color: var(--color-primary); cursor: pointer; } diff --git a/openlibrary/components/lit/OlSelectPopover.js b/openlibrary/components/lit/OlSelectPopover.js index 061120776c3..4c6aaf00a43 100644 --- a/openlibrary/components/lit/OlSelectPopover.js +++ b/openlibrary/components/lit/OlSelectPopover.js @@ -239,7 +239,7 @@ export class OlSelectPopover extends FormAssociatedMixin(LitElement) { width: 16px; height: 16px; margin: 0; - accent-color: var(--primary-blue); + accent-color: var(--color-primary); cursor: pointer; } diff --git a/openlibrary/components/lit/OlShelfButton.js b/openlibrary/components/lit/OlShelfButton.js index 9edb39fc6d3..7705d8d0a86 100644 --- a/openlibrary/components/lit/OlShelfButton.js +++ b/openlibrary/components/lit/OlShelfButton.js @@ -49,6 +49,9 @@ const SHELF_LABEL = { * @prop {Number} shelf - Current shelf id (1–4), or null when on none * @prop {Number} rating - Current rating (1–5), or null. Passed through to the * popover and echoed on every state change + * @prop {String} readDate - Check-in date, whole or partial, shown on the + * popover's Already Read row. Applied by the surface, like shelf and rating + * @prop {Number} eventId - Id of that check-in, so editing the date amends it * @prop {String} userKey - "/people/" when signed in; empty sends the * visitor to log in instead of opening the popover * @prop {String} placement - ol-popover placement for the actions panel; @@ -59,6 +62,8 @@ const SHELF_LABEL = { * * @fires ol-book-state-change - The shelf or rating changed, optimistically or * rolled back. detail: { key, shelf, rating } + * @fires ol-book-check-in - Re-fired from the popover when a finish date is + * saved. detail: { key, date, eventId } */ export class OlShelfButton extends LitElement { static properties = { @@ -68,6 +73,8 @@ export class OlShelfButton extends LitElement { bookTitle: { type: String, attribute: 'book-title' }, shelf: { type: Number }, rating: { type: Number }, + readDate: { type: String, attribute: 'read-date' }, + eventId: { type: Number, attribute: 'event-id' }, userKey: { type: String, attribute: 'user-key' }, placement: { type: String }, labels: { type: Object }, @@ -265,6 +272,8 @@ export class OlShelfButton extends LitElement { .book=${{ key: this.workKey, title: this.bookTitle, editionKey: this.editionKey }} .shelf=${this.shelf} .rating=${this.rating} + .readDate=${this.readDate} + .eventId=${this.eventId} .labels=${this.labels} user-key=${this.userKey} placement=${ifDefined(this.placement)} diff --git a/openlibrary/components/lit/utils/books-api.js b/openlibrary/components/lit/utils/books-api.js index c8f0626b77a..238ace10a5c 100644 --- a/openlibrary/components/lit/utils/books-api.js +++ b/openlibrary/components/lit/utils/books-api.js @@ -74,14 +74,19 @@ export const EVENT = Object.freeze({ START: 1, UPDATE: 2, FINISH: 3 }); * POST /works/OL..W/check-ins — when the reader finished the book. * `month` and `day` are optional: a year alone, or a year and month, are both * valid check-ins, which is what lets the UI offer "in 2026". + * + * `eventId` edits that check-in in place. Without it the server records another + * one, which would count as a second book finished — so pass it whenever the + * reader is changing a date they already gave. */ -export function setCheckIn(workKey, { year, month = null, day = null, editionKey } = {}) { +export function setCheckIn(workKey, { year, month = null, day = null, editionKey, eventId = null } = {}) { return request(`/works/${olid(workKey)}/check-ins`, json({ event_type: EVENT.FINISH, year, month, day, edition_key: editionKey || null, + event_id: eventId || null, })); } diff --git a/openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js b/openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js index 76947bfe26f..87a4b7e500c 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js +++ b/openlibrary/plugins/openlibrary/js/my-books/CreateListForm.js @@ -81,7 +81,8 @@ export class CreateListForm { */ async createNewList() { // Construct seed object for first list item: - const listTitle = websafe(this.listTitleInput.value); + const rawListTitle = this.listTitleInput.value; + const listTitle = websafe(rawListTitle); const listDescription = websafe(this.listDescriptionInput.value); const openDropper = myBooksStore.getOpenDropper(); @@ -103,6 +104,13 @@ export class CreateListForm { // Update all droppers with new list data this.updateDroppersOnListCreation(data['key'], listTitle, data['key']); + // Any popovers on the page cache the list + // set; tell them so their pane shows the new list too. Raw + // name: the popover renders through Lit, which escapes itself. + document.dispatchEvent(new CustomEvent('ol-list-created', { + detail: { key: data['key'], name: rawListTitle, seedKey: seed } + })); + // Clear list creation form fields, nullify seed this.resetForm(); }) diff --git a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js index cedfa9bbb60..703937bda85 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js +++ b/openlibrary/plugins/openlibrary/js/my-books/MyBooksDropper/CheckInComponents.js @@ -88,7 +88,7 @@ export class CheckInComponents { initialize() { this.checkInPrompt.initialize(); - this.checkInPrompt.getRootElement().addEventListener('submit-check-in', (event) => { + this.checkInPrompt.getRootElement()?.addEventListener('submit-check-in', (event) => { const year = event.detail.year; const month = event.detail.month; const day = event.detail.day; @@ -342,17 +342,22 @@ export class CheckInComponents { * Adds functionality to the component containing the "When did you finish this book?" * prompt. * + * Surfaces that ask for the date somewhere else render the container without a + * prompt, so every method here has to survive a missing root element. + * * @class */ class CheckInPrompt { /** - * @param {HTMLElement} checkInPrompt + * @param {HTMLElement|null} checkInPrompt */ constructor(checkInPrompt) { this.rootElem = checkInPrompt; } initialize() { + if (!this.rootElem) return; + const yearLink = this.rootElem.querySelector('.prompt-current-year'); yearLink.addEventListener('click', () => { // Get the current year @@ -388,26 +393,26 @@ class CheckInPrompt { day: day } }); - this.rootElem.dispatchEvent(submitEvent); + this.rootElem?.dispatchEvent(submitEvent); } /** * Hides this check-in prompt. */ hide() { - this.rootElem.classList.add('hidden'); + this.rootElem?.classList.add('hidden'); } /** * Shows this check-in prompt. */ show() { - this.rootElem.classList.remove('hidden'); + this.rootElem?.classList.remove('hidden'); } /** * Returns reference to the root element of this check-in prompt. - * @returns {HTMLElement} + * @returns {HTMLElement|null} */ getRootElement() { return this.rootElem; diff --git a/openlibrary/plugins/openlibrary/js/my-books/index.js b/openlibrary/plugins/openlibrary/js/my-books/index.js index 9d86698e915..b40782d1dcd 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/index.js +++ b/openlibrary/plugins/openlibrary/js/my-books/index.js @@ -4,6 +4,7 @@ import myBooksStore from './store'; import { getListPartials } from '../lists/ListService'; import { ShowcaseItem, createActiveShowcaseItem, toggleActiveShowcaseItems } from '../lists/ShowcaseItem'; import { removeChildren } from '../utils'; +import { websafe } from '../jsdef'; export { initShelfButtons } from './shelf-buttons'; @@ -42,6 +43,19 @@ export function initMyBooksAffordances(dropperElements, showcaseElements) { myBooksStore.setUserKey(userKey); myBooksStore.setDroppers(droppers); + // A mixed page: a list created inside an popover has to + // appear in these droppers too. CreateListForm's own creations dispatch on + // `document` and already update the droppers, so only bubbled events — + // whose target is the popover — are handled here. + document.addEventListener('ol-list-created', (e) => { + if (e.target === document) return; + // The name is patron text and the row is built with innerHTML. + const listTitle = websafe(e.detail.name); + for (const dropper of myBooksStore.getDroppers()) { + dropper.readingLists.onListCreationSuccess(e.detail.key, listTitle, false, ''); + } + }); + getListPartials() .then(response => response.json()) .then((data) => { diff --git a/openlibrary/plugins/openlibrary/js/my-books/shelf-buttons.js b/openlibrary/plugins/openlibrary/js/my-books/shelf-buttons.js index 5e3a1ddff5a..43609a97721 100644 --- a/openlibrary/plugins/openlibrary/js/my-books/shelf-buttons.js +++ b/openlibrary/plugins/openlibrary/js/my-books/shelf-buttons.js @@ -1,22 +1,17 @@ /** * Owns book state for a page of ``s. * - * The buttons are stateless by contract: they never write their own `shelf` or - * `rating`, they emit `ol-book-state-change` — optimistically on click, and - * again with the old value if the write fails — and the surface applies it. - * Server-rendering the attributes only supplies the opening state, so without - * this the label stops matching the server after the first change. + * The buttons are stateless by contract: they never write their own `shelf`, + * `rating` or read date, they emit `ol-book-state-change` — optimistically on + * click, and again with the old value if the write fails — and the surface + * applies it. Server-rendering the attributes only supplies the opening state, + * so without this the label stops matching the server after the first change. * * Applying it centrally is also what keeps two buttons for the same work in - * step, and it is where the check-in prompt gets told what happened: the prompt - * is a sibling of the button (`#check-in-container-`, same id the dropper - * used), so the button stays unaware of check-ins. + * step, including the finish date the popover shows on its Already Read row. * * @module my-books/shelf-buttons */ -import { CheckInComponents } from './MyBooksDropper/CheckInComponents'; - -const ALREADY_READ = 3; /** "/works/OL1W" (or "OL1W") → "OL1W". */ function olid(key) { @@ -29,51 +24,42 @@ function olid(key) { export function initShelfButtons(shelfButtons) { /** @type {Map} */ const buttonsByWork = new Map(); - /** @type {Map} */ - const checkInsByWork = new Map(); for (const button of shelfButtons) { const workOlid = olid(button.getAttribute('work-key')); if (!workOlid) continue; - if (!buttonsByWork.has(workOlid)) { - buttonsByWork.set(workOlid, []); - - // Only rendered for signed-in patrons. - const container = document.querySelector(`#check-in-container-${workOlid}`); - if (container) { - const components = new CheckInComponents(container); - components.initialize(); - checkInsByWork.set(workOlid, components); - } - } + if (!buttonsByWork.has(workOlid)) buttonsByWork.set(workOlid, []); buttonsByWork.get(workOlid).push(button); } if (!buttonsByWork.size) return; - // The event is composed, so one document-level listener covers every button. + /** @param {string} key @param {(button: HTMLElement) => void} apply */ + function forWork(key, apply) { + for (const button of buttonsByWork.get(olid(key)) || []) apply(button); + } + + // The events are composed, so one document-level listener covers every + // button on the page. document.addEventListener('ol-book-state-change', (event) => { const { key, shelf, rating } = event.detail || {}; - const workOlid = olid(key); - - for (const button of buttonsByWork.get(workOlid) || []) { + forWork(key, (button) => { button.shelf = shelf ?? null; button.rating = rating ?? null; - } - - const components = checkInsByWork.get(workOlid); - if (!components) return; - - if (shelf === null || shelf === undefined) { // Coming off a shelf deletes the check-ins server-side. - components.hideCheckInPrompt(); - components.hideCheckInDisplay(); - components.resetForm(); - } else if (shelf === ALREADY_READ && !components.hasReadDate()) { - components.showCheckInPrompt(); - } else { - components.hideCheckInPrompt(); - } + if (shelf === null || shelf === undefined) { + button.readDate = null; + button.eventId = null; + } + }); + }); + + document.addEventListener('ol-book-check-in', (event) => { + const { key, date, eventId } = event.detail || {}; + forWork(key, (button) => { + button.readDate = date ?? null; + button.eventId = eventId ?? null; + }); }); } diff --git a/openlibrary/templates/design/components/book-actions.html.jinja b/openlibrary/templates/design/components/book-actions.html.jinja index d63f9961a36..60df1646706 100644 --- a/openlibrary/templates/design/components/book-actions.html.jinja +++ b/openlibrary/templates/design/components/book-actions.html.jinja @@ -41,7 +41,7 @@ Requests. Shelves post to /works/OL…W/bookshelves.json, ratings to /works/OL…W/ratings.json, check-ins to /works/OL…W/check-ins, lists through /partials/MyBooksDropperLists.json (read) and the lists/seeds endpoints (write). A 401 on any of them redirects to login.

- Check-ins. Choosing Already Read slides in "When did you finish this book?" — because a year on its own is a valid check-in, "In 2026" is one tap, and "Other date" swaps in year/month/day selects where each enables the next. Only an explicit shelf choice opens it: rating a book moves it to Already Read server-side too, and interrupting that would turn one tap into two. + Check-ins. Choosing Already Read slides in "When did you finish this book?" — because a year on its own is a valid check-in, "In 2026" is one tap, and "Other date" discloses year/month/day selects underneath itself — each enabling the next — so the two one-tap answers stay on screen and pressing the row again closes them. Only an explicit shelf choice opens it: rating a book moves it to Already Read server-side too, and interrupting that would turn one tap into two. A date already recorded is marked in the pane rather than left to read as unanswered: whichever row holds it is aria-current, and when no shortcut can express it — a partial date, or any year not offered — "Other date" carries the date and opens on it, selects seeded, so the answer is not hidden behind a collapsed row. The date also rides on the Already Read row, which carries a chevron rather than a check because clicking it goes back to the pane to amend the date — pass event-id alongside read-date so amending edits that check-in instead of recording a second finish.

Panes. The track's width and slide are both derived from the PANES list, so a new pane is an entry there plus a _render* method. Escape in any sub-pane goes back to the main one; a second Escape closes the popover. diff --git a/openlibrary/templates/my_books/check_ins/check_in_prompt.html b/openlibrary/templates/my_books/check_ins/check_in_prompt.html index 21b96ee192c..2a3e7ccce37 100644 --- a/openlibrary/templates/my_books/check_ins/check_in_prompt.html +++ b/openlibrary/templates/my_books/check_ins/check_in_prompt.html @@ -1,4 +1,4 @@ -$def with (work_key, read_status, edition_key=None, last_read_date=None, event_id=None) +$def with (work_key, read_status, edition_key=None, last_read_date=None, event_id=None, show_prompt=True) $# work_key : str : The work key $# read_status : int | None : Number representing which shelf this work is on. @@ -11,6 +11,9 @@ $# edition_key : str | None : The edition key $# last_read_date : str | None : Date that the patron last read the book $# event_id : str | None : ID of record for this event +$# show_prompt : bool : Whether to ask for a date here. False where the surface +$# asks somewhere else — asks inside its own popover — and +$# only the "Read " display and its edit form are wanted. $code: work_olid = work_key.split('/')[-1] @@ -38,16 +41,17 @@

- $ display_prompt = read_status == 3 -
- $_("When did you finish this book?") - - $ year = current_year() - $year - $_("Today") - $_("Other") - -
+ $if show_prompt: + $ display_prompt = read_status == 3 +
+ $_("When did you finish this book?") + + $ year = current_year() + $year + $_("Today") + $_("Other") + +
$if render_once("check-in-form-template"):