starred/audiobookshelf
1
0
Fork 0
mirror of https://github.com/advplyr/audiobookshelf.git synced 2026-05-30 23:40:40 +02:00
Code Issues 1.0k Packages Projects Releases 10 Wiki Activity

[PR #3037] [MERGED] Bookshelf and cards refactoring #3849

New Issue
Closed
opened 2026-04-25 00:17:18 +02:00 by adam · 0 comments
adam commented 2026-04-25 00:17:18 +02:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/advplyr/audiobookshelf/pull/3037
Author: @mikiher
Created: 6/3/2024
Status: ✅ Merged
Merged: 6/25/2024
Merged by: @advplyr

Base: master ← Head: bookshelf-refactor-2

📝 Commits (10+)

  • 2186603 Major bookshelf refactor
  • 651601a Add podcast to supported shelf types
  • 5f8066e Add default line heights converted to em units to tailwind config
  • 3ef189e feat: Add a Show Subtitles option
  • 635f22d Reverted default spacing and font-sizing changes, and extended spaing with em-based variants
  • f541bc2 Fix ItemSlider component on search page
  • 514fb5f Fix ItemSlider component select
  • 090c020 Fix bookshelf view search page showing tags shelf, fix bookshelf row arrow overlay height
  • e06ab59 Revert "feat: Add a Show Subtitles option"
  • 2819317 Remove now-unsued slider components

📊 Changes

26 files changed (+892 additions, -1066 deletions)

View changed files

📝 client/components/app/BookShelfCategorized.vue (+10 -20)
📝 client/components/app/BookShelfRow.vue (+24 -45)
📝 client/components/app/LazyBookshelf.vue (+52 -40)
📝 client/components/cards/AuthorCard.vue (+47 -34)
📝 client/components/cards/GroupCard.vue (+18 -8)
📝 client/components/cards/LazyAlbumCard.vue (+45 -17)
📝 client/components/cards/LazyBookCard.vue (+146 -117)
📝 client/components/cards/LazyCollectionCard.vue (+45 -21)
📝 client/components/cards/LazyPlaylistCard.vue (+32 -19)
📝 client/components/cards/LazySeriesCard.vue (+37 -24)
📝 client/components/cards/NarratorCard.vue (+25 -21)
➖ client/components/widgets/AuthorsSlider.vue (+0 -112)
📝 client/components/widgets/CoverSizeWidget.vue (+2 -2)
➖ client/components/widgets/EpisodeSlider.vue (+0 -161)
📝 client/components/widgets/ItemSlider.vue (+103 -52)
➖ client/components/widgets/NarratorsSlider.vue (+0 -100)
➖ client/components/widgets/SeriesSlider.vue (+0 -109)
📝 client/cypress/tests/components/cards/AuthorCard.cy.js (+22 -17)
➕ client/cypress/tests/components/cards/ItemSlider.cy.js (+85 -0)
📝 client/cypress/tests/components/cards/LazyBookCard.cy.js (+30 -58)

...and 6 more files

📄 Description

This refactoring addresses a number of issues I found while trying to add display of an optional subtitle line (which I left out at the moment):

  • The book card (and other cards) width and height are passed as props
    • This can (and does) cause incosistencies, where various card containers use different values. For example book cards currently have slightly different size on the home and library pages.
  • The height of the card does not reflect the actual card height. the bottom book details use absolute positioning outside the card area.
    • This requires complex layout computation that also happens outside the card, to make the card containers have the correct height to display both the card image and the details below.
  • sizeMultiplier is used many times in each of the cards.
    • Wherever some size attribute is set, it is multiplied by sizeMultiplier. This clutters the code quite a bit.
    • The sizeMultiplier is computed differently in each of the cards, although the result should be the same
  • bookCoverAspectRatio is passed as prop
    • This can also be computed in a single place.
  • There's is a separate itemSlider component for each card type.
    • The Vue templates and code in most component are mostly duplicates.
  • The bookshelf scaling of spacing and font-sizes is inconsistent
    • When the scaling changes (by increasing or decreasing the cover size), some of the spacing and font-sizes change, some don't

The following bookshelf fixes and refactoring were implemented (with the intention of leaving functionality as-is, except for bug fixes):

  • The height and width of bookshelf cards is now determined by default inside the card
    • The height of the card graphics (cover image or material icon) has a default.
    • All other properties are computed.
    • the card computed height now includes the bottom description if it has one.
      • one exception to this is the description in standard view, which needs to appear on top of the shelf divider
  • The shelf height is now determined by the computed height of the cards inside it
    • In lazyBookshelf, where the height needs to be determined in advance, one dummy card of the right type is mounted in order to get its width and height, and then removed.
    • This makes shelf height calculation straightforward and less error-prone.
  • All the itemSliders are now merged into a single component
    • The card component is dynamically determined by the shelf type.
    • Also fixed some small edge case slider scrolling issue, and made it not dependent on the card width
  • sizeMultiplier and bookCoverAspect ratio are now obtained from the store
  • The scaling and font sizes is now made consistent by changing them to relative em units
    • This is mostly done transparently by changing the default tailwind definitions
    • A version of the original tailwind spacing is still available (by adding r to the spacing number, e.g. right-4r)
    • At the bookshelf level, the font-size is set to sizeMultiplier * 1rem
      • this makes the tailwind classes inside the bookshelf reactive to the sizeMultplier (since they are now specified in em units)
      • this also makes sizeMultiplier multiplication redundant in most places (by changing rem units to em), making the code less cluttered.
  • Fixed some buggy booksPerFetch behavior
    • When cover size is set to smallest, lazyBookshelf doesn't get enough books to fill the first page, so something like this appears:
      Screenshot 2024-06-02 085102 This was due to booksPerFetch having static values. It was fixed to have a value depending on the the number of books per shelf and the number of shelfs per page.

I've tested this quite extensively in library, search, and home pages, in both stadard and detail view, while changing cover sizes back and forth. I believe all is working as intended now.

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/advplyr/audiobookshelf/pull/3037 **Author:** [@mikiher](https://github.com/mikiher) **Created:** 6/3/2024 **Status:** ✅ Merged **Merged:** 6/25/2024 **Merged by:** [@advplyr](https://github.com/advplyr) **Base:** `master` ← **Head:** `bookshelf-refactor-2` --- ### 📝 Commits (10+) - [`2186603`](https://github.com/advplyr/audiobookshelf/commit/2186603039ace746638471ab05c182a088f8123c) Major bookshelf refactor - [`651601a`](https://github.com/advplyr/audiobookshelf/commit/651601adf6f1de3de42c24a95fb6ef1f00d9526a) Add podcast to supported shelf types - [`5f8066e`](https://github.com/advplyr/audiobookshelf/commit/5f8066e6018cf4d707d908830932b9d07c87f760) Add default line heights converted to em units to tailwind config - [`3ef189e`](https://github.com/advplyr/audiobookshelf/commit/3ef189ed4a6edb4be6deaeb01da381644dbdaaee) feat: Add a Show Subtitles option - [`635f22d`](https://github.com/advplyr/audiobookshelf/commit/635f22ddfe7a2457eef052717316275329065bea) Reverted default spacing and font-sizing changes, and extended spaing with em-based variants - [`f541bc2`](https://github.com/advplyr/audiobookshelf/commit/f541bc2159336e35a861e0dc210988c46de9b9b3) Fix ItemSlider component on search page - [`514fb5f`](https://github.com/advplyr/audiobookshelf/commit/514fb5f7dadb934f53e7238e0047af7c486c8cd5) Fix ItemSlider component select - [`090c020`](https://github.com/advplyr/audiobookshelf/commit/090c02079d697b68ab1171534b1d2d216a364287) Fix bookshelf view search page showing tags shelf, fix bookshelf row arrow overlay height - [`e06ab59`](https://github.com/advplyr/audiobookshelf/commit/e06ab594e121bff280ce2d9b697ca1e63926a0dd) Revert "feat: Add a Show Subtitles option" - [`2819317`](https://github.com/advplyr/audiobookshelf/commit/2819317924625c0fd72e0b48d0fa3a21d1af83bd) Remove now-unsued slider components ### 📊 Changes **26 files changed** (+892 additions, -1066 deletions) <details> <summary>View changed files</summary> 📝 `client/components/app/BookShelfCategorized.vue` (+10 -20) 📝 `client/components/app/BookShelfRow.vue` (+24 -45) 📝 `client/components/app/LazyBookshelf.vue` (+52 -40) 📝 `client/components/cards/AuthorCard.vue` (+47 -34) 📝 `client/components/cards/GroupCard.vue` (+18 -8) 📝 `client/components/cards/LazyAlbumCard.vue` (+45 -17) 📝 `client/components/cards/LazyBookCard.vue` (+146 -117) 📝 `client/components/cards/LazyCollectionCard.vue` (+45 -21) 📝 `client/components/cards/LazyPlaylistCard.vue` (+32 -19) 📝 `client/components/cards/LazySeriesCard.vue` (+37 -24) 📝 `client/components/cards/NarratorCard.vue` (+25 -21) ➖ `client/components/widgets/AuthorsSlider.vue` (+0 -112) 📝 `client/components/widgets/CoverSizeWidget.vue` (+2 -2) ➖ `client/components/widgets/EpisodeSlider.vue` (+0 -161) 📝 `client/components/widgets/ItemSlider.vue` (+103 -52) ➖ `client/components/widgets/NarratorsSlider.vue` (+0 -100) ➖ `client/components/widgets/SeriesSlider.vue` (+0 -109) 📝 `client/cypress/tests/components/cards/AuthorCard.cy.js` (+22 -17) ➕ `client/cypress/tests/components/cards/ItemSlider.cy.js` (+85 -0) 📝 `client/cypress/tests/components/cards/LazyBookCard.cy.js` (+30 -58) _...and 6 more files_ </details> ### 📄 Description This refactoring addresses a number of issues I found while trying to add display of an optional subtitle line (which I left out at the moment): - The book card (and other cards) width and height are passed as props - This can (and does) cause incosistencies, where various card containers use different values. For example book cards currently have slightly different size on the home and library pages. - The height of the card does not reflect the actual card height. the bottom book details use absolute positioning outside the card area. - This requires complex layout computation that also happens outside the card, to make the card containers have the correct height to display both the card image and the details below. - `sizeMultiplier` is used *many* times in each of the cards. - Wherever some size attribute is set, it is multiplied by `sizeMultiplier`. This clutters the code quite a bit. - The `sizeMultiplier` is computed differently in each of the cards, although the result should be the same - `bookCoverAspectRatio` is passed as prop - This can also be computed in a single place. - There's is a separate itemSlider component for each card type. - The Vue templates and code in most component are mostly duplicates. - The bookshelf scaling of spacing and font-sizes is inconsistent - When the scaling changes (by increasing or decreasing the cover size), some of the spacing and font-sizes change, some don't The following bookshelf fixes and refactoring were implemented (with the intention of leaving functionality as-is, except for bug fixes): - The height and width of bookshelf cards is now determined by default inside the card - The height of the card graphics (cover image or material icon) has a default. - All other properties are computed. - the card computed height now includes the bottom description if it has one. - one exception to this is the description in standard view, which needs to appear _on top_ of the shelf divider - The shelf height is now determined by the computed height of the cards inside it - In lazyBookshelf, where the height needs to be determined in advance, one dummy card of the right type is mounted in order to get its width and height, and then removed. - This makes shelf height calculation straightforward and less error-prone. - All the itemSliders are now merged into a single component - The card component is dynamically determined by the shelf type. - Also fixed some small edge case slider scrolling issue, and made it not dependent on the card width - sizeMultiplier and bookCoverAspect ratio are now obtained from the store - The scaling and font sizes is now made consistent by changing them to relative em units - This is mostly done transparently by changing the default tailwind definitions - A version of the original tailwind spacing is still available (by adding r to the spacing number, e.g. right-4r) - At the bookshelf level, the font-size is set to sizeMultiplier * 1rem - this makes the tailwind classes inside the bookshelf reactive to the sizeMultplier (since they are now specified in em units) - this also makes sizeMultiplier multiplication redundant in most places (by changing rem units to em), making the code less cluttered. - Fixed some buggy booksPerFetch behavior - When cover size is set to smallest, lazyBookshelf doesn't get enough books to fill the first page, so something like this appears: <img width="1917" alt="Screenshot 2024-06-02 085102" src="https://github.com/mikiher/audiobookshelf/assets/22557398/54c238e5-b71d-49d0-9052-b4aaec6fee07"> This was due to booksPerFetch having static values. It was fixed to have a value depending on the the number of books per shelf and the number of shelfs per page. I've tested this quite extensively in library, search, and home pages, in both stadard and detail view, while changing cover sizes back and forth. I believe all is working as intended now. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
adam added the pull-request label 2026-04-25 00:17:18 +02:00
adam closed this issue 2026-04-25 00:17:18 +02:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
authentication
backlog
bug
chapter editor
config-issue
ebooks
encoding/embedding
enhancement
help wanted
listening sessions & progress
planned
possible plugin
progress sync
pull-request
Mirrored from GitHub Pull Request
 sorting/filtering/searching
unable to reproduce
upload
users & permissions
waiting
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
adam (Adam Melkus)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/audiobookshelf#3849
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?