Adopt ol-chip for subject tags, author page, and publisher page#12261
Adopt ol-chip for subject tags, author page, and publisher page#12261lokesh merged 7 commits intointernetarchive:masterfrom
Conversation
Co-author-by: Evelyn Pham <phamphuonglinh250104@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adopts the ol-chip web component for subject-tag displays to align UI/UX with existing chip usage (e.g., search facets / related subjects) and improve consistency across book, author, and publisher pages.
Changes:
- Updated
SubjectTagsmacro to render subjects as<ol-chip>elements. - Updated author and publisher templates to render subject facets as
<ol-chip>within<ol-chip-group>. - Added CSS intended to improve header alignment in clamped link-box layouts.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| static/css/components/link-box.css | Adds styling meant for clamped link-box headers. |
| openlibrary/templates/type/author/view.html | Renders “subjects by author” facets using ol-chip-group + ol-chip. |
| openlibrary/templates/publishers/view.html | Renders “common subjects” using ol-chip-group + ol-chip. |
| openlibrary/macros/SubjectTags.html | Renders work/edition subject tags using ol-chip. |
…e subjects Co-authored-by: Evelyn Pham <phamphuonglinh250104@gmail.com>
linhkhanhhoang
force-pushed
the
12158/adopt_ol_chip
branch
from
582a1ff to
04bedd7
Compare
for more information, see https://pre-commit.ci
|
Everything looks great, thanks for your help with this. 🙏
I've started a Manual Testing section in our wiki which should help with this: https://docs.openlibrary.org/developers/tools/testing.html#manual-testing There are still some challenging in building up local data so the testing better reflects the live site experience. |
… in link-box - Use composedPath() in clampers.js so clicks on elements inside anchor tags (e.g. ol-chip inside <a>) are correctly detected - Add Chip Group section to the design page with gap and wrapping demos - Add spacing for ol-chip inside .link-box .clamp
for more information, see https://pre-commit.ci
|
|
||
| clamper.addEventListener('click', (event) => { | ||
| if (event.target instanceof HTMLAnchorElement) { | ||
| if (event.composedPath().some(el => el instanceof HTMLAnchorElement)) { |
There was a problem hiding this comment.
This prevents the clamper from toggling when a link is clicked, even if it is clicked in the shadow dom.
| </script> | ||
| </section> | ||
| </article> | ||
| <article class="design-pattern" id="chip-group"> |
There was a problem hiding this comment.
Added ol-chip-group examples for future devs. They will be visible at: https://openlibrary.org/developers/design
|
Testing Summary I completed manual testing for the WorkInfo macro, Tag page, and List page subjects changes in this PR with the following approach: 1. WorkInfo Macro:
$if query_param('legacy_workinfo'):
$:macros.WorkInfo(work)
2. Tag Page:
3. List Page:
if False → if True
All testing completed. Ready for review. @lokesh |
|
@evelynpham04 thanks for all the testing from your side. I actually added the Sorry for the confusion about the testing tag, though more testing never hurts. Should go out with the next deploy next week. |
Closes #12158
This PR adopted the ol-chip component into the SubjectTags macro, the Author page — subjects by author, the Publisher page — common subjects, Search result subject highlights, WorkInfo macro, Tag page, and List page subjects.
Technical
For the SubjectTags macro, we didn't use because when we did, it messed up the expand/clamp part, and it wouldn't show any subjects unless you clicked the toggle.
Also, we didn't know how to test for the WorkInfo macro, Tag page, and List page subjects changes because we couldn't find where they are located.
Testing
Screenshot
SubjectTags macro
The Author page — subjects by author
the Publisher page — common subjects
Search result subject highlights
Stakeholders
@lokesh