feat: Add 'Read More' - PART 2

[lokesh]

Continuation of the work done in #11666

This PR finishes replacing all instances of the Read More feature with the new web component. It also removes the code related to the old component.

Technical

Extends usage to additional locations:

• Table of Contents and Work description in edition view
• Chapter highlights in search results

Removes old Read More implementation:

• Deleted openlibrary/macros/ReadMore.html macro
• Deleted static/css/components/read-more.less styles
• Deleted openlibrary/plugins/openlibrary/js/readmore.js component
• Deleted tests/unit/js/readmore.test.js tests

Testing

1. Navigate to an edition page with a long description (e.g., /books/OL...)

• Verify description is truncated to ~4 lines with "Read More" button
• Click to expand/collapse, confirm toggle works

2. Navigate to an edition page with a Table of Contents

• Verify TOC is truncated with "Read More" button when content exceeds 200px
• Toggle to expand/collapse

Screenshot

Stakeholders

@RayBB @mekarpeles @cdrini @jimchamp

[cdrini]

Seeing some blocking issues for the TOC on the search page:

https://testing.openlibrary.org/search?q=java+tree&mode=everything

Before:

---

I like the idea of making the leader dots in the TOC lighter; it makes the overall page look much cleaner! There's still something a bit funny about this UX; it's causing my eyes to strain when I see it; causing a kind of funny visual banding effect on desktop. It's a bit more accentuated now that the font size is smaller and the lines are fainter. Not sure what solution is! Not a blocker.

[lokesh]

@cdrini the Search ToC primary issue that you are referring to is the background color of the new Read More component?

[cdrini]

Yes exactly! It should look like the original designs, the change in colours/font size/padding make it look broken currently. TOC in search is a new feature that was just recently released, so we need to be more mindful of regressions, to make sure we lead with our best foot forward. It's also a great case-study in some of the difficulties that web component CSS isolation causes ; we do want our web components to support being styled to match their context. We should be able to achieve the same UI with adding some ::parts to the read more component or css custom props.

[lokesh]

Fixed rendering:

It's also a great case-study in some of the difficulties that web component CSS isolation causes ; we do want our web components to support being styled to match their context. We should be able to achieve the same UI with adding some ::parts to the read more component or css custom props.

I've added additional properties to support this variant of the component. In general, I'd lean into properties over exposing ::parts, though the latter could be useful in some instances. But one of the big benefits of web components is predictability of rendering, and allowing devs to arbitrarily style and tinker exposes us to maintenance challenges. Better to start as narrow as possible as it is easy to add features, but they are hard to remove.

[cdrini]

Nice lgtm! Merging as-is, but I think we might want to brainstorm about the styling interface during our call a bit. Also there's one small non-blocking bug with the TOC in search when in librarian mode, where clicking "show more" causes the search item card to become selected. I think the code was previously ignoring clicks on anchor elements.

This looks great!! 🥳 Love all the deleted/DRY'd up code!

[cdrini]

I'm not keen on these being separate properties on the web component (eg <ol-read-more padding-left="4px">); that's a pattern I haven't seen anywhere before. It blurs the separation between styling and html content, and adds a decent amount of JS for stuff like parsing values, and creates a syntax slightly separate from CSS that consumers have to learn per-component. Not a blocker, but I think this is something we might want to discuss on Friday to get folks' thoughts on. CC @RayBB

Patterns I have seen elsewhere (eg PrimeVue, webawesome) are:

CSS custom properties are the public interface

<ol-read-more style="--ol-read-more-pading-left: 4px"></ol-read-more>

Or

ol-read-more {
    --ol-read-more-padding-left: 4px;
}

CSS parts are the public interface

ol-read-more::part(toggle-btn) {
    padding-left: 4px;
}

I'm partial to using custom properties for themeing/styling that applies everywhere, and exposing every "chunk" of a component as a ::part for further config. I think that strikes a good compromise between having controlled interfaces where we can easily make updates to a component's internals without having to maintain a huge public API / breaking clients, while also allowing clients to configure the component without them having to modify the component every time a new use case pops up to expose new hooks. A balance of flexibility and rigidity/brittleness.

[lokesh]

@cdrini thanks for sharing alternative directions as it illuminates the deficiencies with the padding left property. I think the css ::part is prob the right approach.

I'll get a PR ready ahead of our meeting week, it will also be a good opportunity to discuss our strategy for when to expose these css hooks . Our strategy doesn't have be to perfect, as web component usage is minimal in the codebase, and we have time to dial it in as we learn through experience. Thanks again for the feedback.

[RayBB]

@lokesh what a huge win! I love seeing a PR like this knocking out hundreds of old lines of code.

Excited to see what's next!