[docs] Improve ad display #21219
[docs] Improve ad display #21219
Conversation
|
No bundle size changes comparing d49be19...ca9f542 |
| @@ -33,17 +33,26 @@ const useStyles = makeStyles((theme) => ({ | |||
| }, | |||
| })); | |||
|
|
|||
| // Persisted for the whole session | |||
| const random = Math.random(); | |||
There was a problem hiding this comment.
Why do we not want to cycle through the diamond sponsors while browsing?
There was a problem hiding this comment.
The logic avoids layout jumps for a given session.
There was a problem hiding this comment.
Could you include this in the comment? You're talking about re-mounting in the navbar which can be fixed. Please be specific since we don't do this for Ads.
There was a problem hiding this comment.
I have added this in the comment. Also, we will increase the display split to 25% starting next month (up from 5%).
There was a problem hiding this comment.
Layout jumps where? The question you're trying to answer is why this doesn't apply to Ad
There was a problem hiding this comment.
The logo doesn't reserve any space. When we decide to display it, we move the menu items down by 35px. The objective is to avoid this repeated jump down and up.
I don't see the link with the Ad component. There, we always display an Ad. We have a few random usage for increasing the cardinality of ads people see.
There was a problem hiding this comment.
There, we always display an Ad.
But we cycle through them so it's not immediately clear they don't cause a layout shift as well.
I thought this variable decides which diamond sponsor we display not that we display one.
Maybe add a descriptive name for the variable so that it's obvious what this is used for? In the current form it can be used for anything "random" which is most the time the wrong semantics.
There was a problem hiding this comment.
Yeah ok, changing the name
A collection of small changes.