fix: full text content flashes before being truncated with <ol-read-more>#11812
fix: full text content flashes before being truncated with <ol-read-more>#11812lokesh wants to merge 5 commits intointernetarchive:masterfrom
Conversation
lokesh
added
the
Theme: Design
openlibrary/templates/site/head.html
Outdated
| /* Hide ol-read-more content before component is defined to prevent flash of unstyled content */ | ||
| ol-read-more:not(:defined) { | ||
| visibility: hidden; | ||
| height: 0; |
There was a problem hiding this comment.
The main issue isn't the flash of unstyled content, but the cumulative layout shift, since the height will change while loading, causing all the items after it to change position. Setting the height to 0 here will still cause that issue. I think we might need to set the height explicitly in the element, eg <ol-read-more max-height="80px" style="max-height: 80px"> . Not satisfying, but let's us (1) decide the size at the moment the read-more is created, and (2) have it entirely avoid the layout shift.
There was a problem hiding this comment.
Gotcha, I updated the PR to set a min-height. Now just like production, there isn't a layout shift when the text is truncated. When the text is smaller then the defined height, just like production, there is a small layout shift, though in this updated version, the toggle button doesn't temporarily flash visible.
I also set defaults so that in most cases, you don't need to specify the max-height on the component or apply a min-height inline style.
Last change, I remove the max-lines property from the component. It was not being used and would introduce more complexity in terms of how we would resolve layout shifting.
…cation and updating default max-height - Removed support for line-based truncation in the OLReadMore component. - Updated documentation to clarify usage of height-based truncation with a default max-height of 80px. - Adjusted related HTML templates to reflect changes in the component's attributes and styles for improved consistency.
cdrini
left a comment
cdrini
left a comment
There was a problem hiding this comment.
Lgtm, CLS is resolved! One small tweak to reduce the FOUC, and one question.
| <h3>$_("Table of Contents")</h3> | ||
| $ component_times['TableOfContents'] = time() | ||
| <ol-read-more max-height="200px" more-text="$_('Read More')" less-text="$_('Read Less')"> | ||
| <ol-read-more max-height="200px" style="min-height: 241px" more-text="$_('Read More')" less-text="$_('Read Less')"> |
There was a problem hiding this comment.
This is a bit finnicky since we have to guesstimate the size of the read more button. We could potentially make the the readmore button position absolute inside the container? That way the max-height will define the full height of the component, including the read more button. Not a blocker.
| ol-read-more { | ||
| min-height: 121px; | ||
| overflow: hidden; | ||
| } | ||
| ol-read-more[label-size="small"] { | ||
| min-height: 107px; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
Oh wait the CLS is still there! Should this max-height? And only defined for :not(:defined)? I found this CSS did the trick.
| ol-read-more { | |
| min-height: 121px; | |
| overflow: hidden; | |
| } | |
| ol-read-more[label-size="small"] { | |
| min-height: 107px; | |
| } | |
| </style> | |
| ol-read-more:not(:defined) { | |
| display: block; | |
| max-height: 121px; | |
| overflow: hidden; | |
| } | |
| ol-read-more[label-size="small"]:not(:defined) { | |
| min-height: 107px; | |
| } | |
| </style> |
Also handle no js environments
|
Reverted back to f0f85c1 Limited layout shifting or flashing |
github-actions
bot
added
the
Needs: Response
2 participants
fix: full text content flashes before being truncated with
Technical
Adds inline CSS in head.html to prevent flash of unstyled content for the ol-read-more custom element before its JavaScript definition runs.
Opted to use the
visibility: hiddenvsdisplay: noneas the browser may defer child content preparation if display none is used. Either solution probably fine though.Testing
Screenshot
Fixed:
https://github.com/user-attachments/assets/313ac5b5-e212-4a2e-9f52-1c686ebbccf7
This change also fixes the flash of the 'Read more' button when it is not necessary. This issue is live on production:
https://github.com/user-attachments/assets/186afb95-7578-4356-ad09-f64f180fa13b
Stakeholders
@cdrini