-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Reader: add placeholder for reader-related-card-v2 #8334
Conversation
@@ -60,6 +91,10 @@ export function RelatedPostCard( { post, site, onPostClick = noop, onSiteClick = | |||
'has-thumbnail': !! featuredImage | |||
} ); | |||
|
|||
//if ( ! post || post.state == 'pending' ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this commented out?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For testing only - see todo item above :)
Uncomment logic to only use placeholder when post is missing or still in pending state
Uncommented now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:face-palm:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i have much to learn
@@ -442,3 +442,40 @@ $reader-related-card-v2-breakpoint-small: "( max-width: 535px )"; | |||
max-height: none; | |||
} | |||
} | |||
|
|||
// Placeholders | |||
.reader-related-card-v2.is-placeholder { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to nest the .is-modifier
instead of the selectors? I'll update it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jancavan!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bluefuton Actually nvm. It doesn't make sense in this case. I tried and we're going to end up with too many .is-modifers
.
The stacked RPs have placeholder images too wide: I can look into this @bluefuton. |
@bluefuton Fixed in: 8301f12 |
Thank you @jancavan! |
LGTM 🚢 |
Add a placeholder when related posts blocks are loading in the new Reader full post view.
This will help us when we need to scroll to a specific spot on the page before everything's loaded (e.g. for comments anchor #7892).
Outstanding tasks:
pending
state