Skip to content

Commit

Permalink
[docs-infra] Minimize ad layout shift on mobile (#40582)
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari authored Jan 14, 2024
1 parent e91b96e commit 9e0af1b
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 12 deletions.
14 changes: 8 additions & 6 deletions docs/src/modules/components/Ad.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,8 @@ class AdErrorBoundary extends React.Component {
export const AD_MARGIN_TOP = 3;
export const AD_MARGIN_BOTTOM = 3;
export const AD_HEIGHT = 126;
// Add more height on mobile as the text tends to wrap beyond the image height.
export const AD_HEIGHT_MOBILE = 126 + 16;

// https://stackoverflow.com/a/20084661
function isBot() {
Expand Down Expand Up @@ -214,25 +216,25 @@ export default function Ad() {
return (
<Box
component="span"
sx={{
sx={(theme) => ({
position: 'relative',
display: 'block',
mt: AD_MARGIN_TOP,
mb: AD_MARGIN_BOTTOM,
...(adShape === 'image' && {
minHeight: AD_HEIGHT_MOBILE,
[theme.breakpoints.up('sm')]: {
minHeight: AD_HEIGHT,
}),
},
...(adShape === 'image' && {}),
...(adShape === 'inline' && {
minHeight: AD_HEIGHT,
display: 'flex',
alignItems: 'flex-end',
}),
...(adShape === 'inline2' && {
minHeight: AD_HEIGHT,
display: 'flex',
alignItems: 'flex-end',
}),
}}
})}
data-ga-event-category="ad"
data-ga-event-action="click"
data-ga-event-label={eventLabel}
Expand Down
1 change: 1 addition & 0 deletions docs/src/modules/components/AdCarbon.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const CarbonRoot = styled('span')(({ theme }) => {
const styles = adStylesObject['body-image'](theme);

return {
width: '100%',
'& > div': {
// The isolation logic of carbonads is broken.
// Once the script starts loading, it will asynchronous resolve, with no way to stop it.
Expand Down
17 changes: 14 additions & 3 deletions docs/src/modules/components/AppLayoutDocs.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ import AppTableOfContents from 'docs/src/modules/components/AppTableOfContents';
import AdManager from 'docs/src/modules/components/AdManager';
import AppLayoutDocsFooter from 'docs/src/modules/components/AppLayoutDocsFooter';
import BackToTop from 'docs/src/modules/components/BackToTop';
import { AD_MARGIN_TOP, AD_HEIGHT, AD_MARGIN_BOTTOM } from 'docs/src/modules/components/Ad';
import {
AD_MARGIN_TOP,
AD_HEIGHT,
AD_HEIGHT_MOBILE,
AD_MARGIN_BOTTOM,
} from 'docs/src/modules/components/Ad';

const TOC_WIDTH = 242;

Expand Down Expand Up @@ -62,16 +67,22 @@ const StyledAppContainer = styled(AppContainer, {
? {
'&& .component-tabs .MuiTabs-root': {
// 40px matches MarkdownElement h2 margin-top.
marginBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT}px + 40px)`,
marginBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT_MOBILE}px + 40px)`,
[theme.breakpoints.up('sm')]: {
marginBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT}px + 40px)`,
},
},
'&& .component-tabs.ad .MuiTabs-root': {
marginBottom: 0,
},
}
: {
'&& .description': {
paddingBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT}px)`,
marginBottom: theme.spacing(AD_MARGIN_BOTTOM),
paddingBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT_MOBILE}px)`,
[theme.breakpoints.up('sm')]: {
paddingBottom: `calc(${theme.spacing(AD_MARGIN_TOP)} + ${AD_HEIGHT}px)`,
},
},
'&& .description.ad': {
paddingBottom: 0,
Expand Down
10 changes: 8 additions & 2 deletions docs/src/modules/components/ad.styles.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ const adBodyImageStyles = (theme) => ({
overflow: 'hidden',
border: '1px dashed',
borderColor: theme.palette.divider,
padding: '12px 12px 12px calc(12px + 130px)',
borderRadius: theme.shape.borderRadius,
padding: '8px 8px 8px calc(8px + 130px)',
[theme.breakpoints.up('sm')]: {
padding: '12px 12px 12px calc(12px + 130px)',
},
},
imgWrapper: {
float: 'left',
Expand All @@ -24,7 +27,10 @@ const adBodyImageStyles = (theme) => ({
textDecoration: 'none',
},
description: {
...theme.typography.body1,
...theme.typography.body2,
[theme.breakpoints.up('sm')]: {
...theme.typography.body1,
},
display: 'block',
marginLeft: theme.spacing(1.5),
},
Expand Down
2 changes: 1 addition & 1 deletion docs/translations/translations.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"adblock": "If you don't mind tech-related ads (no tracking or remarketing), and want to keep us running, please whitelist MUI in your blocker.",
"adblock": "If you don't mind tech-related ads (no tracking or remarketing), and want to keep us running, please whitelist us in your blocker.",
"api-docs": {
"componentName": "Component name",
"componentsApi": "Components API",
Expand Down

0 comments on commit 9e0af1b

Please sign in to comment.