Merge pull request #2699 from owid/grapher-redesign-bug-fixes

Grapher redesign: Bug fixes

packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.scss

@@ -59,6 +59,10 @@ $controlRowHeight: 32px; // keep in sync with CONTROLS_ROW_HEIGHT
 &.GrapherComponent.optimizeForHorizontalSpace .relatedQuestion {
     border-top: 0;
 
+    // adds a top border that stretches across the entire page.
+    // since we don't know the width of the page, we use a large number (200vw)
+    // and offset it by another large number (-50vw) to make sure the border
+    // stretches across the entire page.
     &::before {
         content: "";
         position: absolute;

packages/@ourworldindata/grapher/src/captionedChart/CaptionedChart.tsx

@@ -593,9 +593,13 @@ export class StaticCaptionedChart extends CaptionedChart {
     }
 }
 
+// Although a bit unconventional, adding vertical space as a <div />
+// makes margin collapsing impossible and makes it easier to track the
+// space available for the chart area (see the CaptionedChart's `chartHeight` method)
 function VerticalSpace({ height }: { height: number }): JSX.Element {
     return (
         <div
+            className="VerticalSpace"
             style={{
                 height,
                 width: "100%",

packages/@ourworldindata/grapher/src/controls/ShareMenu.scss

@@ -50,9 +50,9 @@ $zindex-ControlsFooter: 2;
             color: $active-text;
         }
 
-        > svg {
+        > .icon {
             margin-right: 6px;
-            position: relative;
+            flex: 0 0 16px;
         }
     }
 }

packages/@ourworldindata/grapher/src/controls/ShareMenu.tsx

@@ -157,7 +157,10 @@ export class ShareMenu extends React.Component<ShareMenuProps, ShareMenuState> {
                     href={twitterHref}
                     rel="noopener"
                 >
-                    <FontAwesomeIcon icon={faXTwitter} /> X
+                    <span className="icon">
+                        <FontAwesomeIcon icon={faXTwitter} />
+                    </span>{" "}
+                    X/Twitter
                 </a>
                 <a
                     target="_blank"
@@ -166,32 +169,43 @@ export class ShareMenu extends React.Component<ShareMenuProps, ShareMenuState> {
                     href={facebookHref}
                     rel="noopener"
                 >
-                    <FontAwesomeIcon icon={faFacebook} /> Facebook
+                    <span className="icon">
+                        <FontAwesomeIcon icon={faFacebook} />
+                    </span>{" "}
+                    Facebook
                 </a>
                 <a
                     className="embed"
                     title="Embed this visualization in another HTML document"
                     data-track-note="chart_share_embed"
                     onClick={this.onEmbed}
                 >
-                    <FontAwesomeIcon icon={faCode} /> Embed
+                    <span className="icon">
+                        <FontAwesomeIcon icon={faCode} />
+                    </span>{" "}
+                    Embed
                 </a>
                 {"share" in navigator && (
                     <a
                         title="Share this visualization with an app on your device"
                         data-track-note="chart_share_navigator"
                         onClick={this.onNavigatorShare}
                     >
-                        <FontAwesomeIcon icon={faShareAlt} /> Share via&hellip;
+                        <span className="icon">
+                            <FontAwesomeIcon icon={faShareAlt} />
+                        </span>{" "}
+                        Share via&hellip;
                     </a>
                 )}
-                {this.state.canWriteToClipboard && (
+                {true && (
                     <a
                         title="Copy link to clipboard"
                         data-track-note="chart_share_copylink"
                         onClick={this.onCopyUrl}
                     >
-                        <FontAwesomeIcon icon={faLink} />
+                        <span className="icon">
+                            <FontAwesomeIcon icon={faLink} />
+                        </span>{" "}
                         {this.state.copied ? "Copied!" : "Copy link"}
                     </a>
                 )}
@@ -202,7 +216,10 @@ export class ShareMenu extends React.Component<ShareMenuProps, ShareMenuState> {
                         href={editUrl}
                         rel="noopener"
                     >
-                        <FontAwesomeIcon icon={faEdit} /> Edit
+                        <span className="icon">
+                            <FontAwesomeIcon icon={faEdit} />
+                        </span>{" "}
+                        Edit
                     </a>
                 )}
             </div>

packages/@ourworldindata/grapher/src/core/Grapher.tsx

@@ -92,6 +92,7 @@ import {
     DEFAULT_GRAPHER_FRAME_PADDING,
     DEFAULT_GRAPHER_ENTITY_TYPE,
     DEFAULT_GRAPHER_ENTITY_TYPE_PLURAL,
+    GRAPHER_DARK_TEXT,
 } from "../core/GrapherConstants"
 import Cookies from "js-cookie"
 import {
@@ -1231,7 +1232,7 @@ export class Grapher
                     this.idealBounds.width - 2 * this.framePaddingHorizontal,
                 lineHeight: 1.2,
                 style: {
-                    fill: "#5b5b5b",
+                    fill: GRAPHER_DARK_TEXT,
                 },
             })
         })
@@ -2681,6 +2682,13 @@ export class Grapher
         )
     }
 
+    @computed get isOnCanonicalUrl(): boolean {
+        if (!this.canonicalUrl) return false
+        return (
+            getWindowUrl().pathname === Url.fromURL(this.canonicalUrl).pathname
+        )
+    }
+
     @computed get embedUrl(): string | undefined {
         return this.manager?.embedDialogUrl ?? this.canonicalUrl
     }

packages/@ourworldindata/grapher/src/core/GrapherConstants.ts

@@ -34,6 +34,8 @@ export const DEFAULT_GRAPHER_HEIGHT = 600
 export const DEFAULT_GRAPHER_FRAME_PADDING = 16
 export const STATIC_EXPORT_DETAIL_SPACING = 8
 
+export const GRAPHER_DARK_TEXT = "#5b5b5b"
+
 export enum CookieKey {
     isAdmin = "isAdmin",
 }

packages/@ourworldindata/grapher/src/core/grapher.scss

@@ -172,6 +172,10 @@ $zindex-controls-drawer: 140;
 .GrapherComponent.optimizeForHorizontalSpace {
     box-shadow: none;
 
+    // adds top and bottom borders that stretch across the entire page.
+    // since we don't know the width of the page, we use a large number (200vw)
+    // and offset it by another large number (-50vw) to make sure the borders
+    // stretch across the entire page.
     &::before,
     &::after {
         content: "";

packages/@ourworldindata/grapher/src/dataTable/DataTable.tsx

@@ -1002,7 +1002,7 @@ function SortIcon(props: {
                     <FontAwesomeIcon icon={faArrowUpLong} />
                     <FontAwesomeIcon
                         icon={faArrowDownLong}
-                        style={{ marginLeft: "-4px" }}
+                        style={{ marginLeft: "-2px" }}
                     />
                 </span>
             )}

packages/@ourworldindata/grapher/src/footer/Footer.tsx

@@ -8,11 +8,15 @@ import {
     DEFAULT_BOUNDS,
     getRelativeMouse,
     MarkdownTextWrap,
+    DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID,
 } from "@ourworldindata/utils"
 import { Tooltip } from "../tooltip/Tooltip"
 import { FooterManager } from "./FooterManager"
 import { ActionButtons } from "../controls/ActionButtons"
-import { DEFAULT_GRAPHER_FRAME_PADDING } from "../core/GrapherConstants"
+import {
+    DEFAULT_GRAPHER_FRAME_PADDING,
+    GRAPHER_DARK_TEXT,
+} from "../core/GrapherConstants"
 
 /*
 
@@ -414,7 +418,8 @@ export class Footer<
                     onClick={action(() => {
                         // on data pages, scroll to the "Sources and Processing" section
                         // on grapher pages, open the sources modal
-                        const sourcesIdOnDataPage = "sources-and-processing"
+                        const sourcesIdOnDataPage =
+                            DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID
                         const sourcesElement =
                             document.getElementById(sourcesIdOnDataPage)
                         if (sourcesElement && sourcesElement.scrollIntoView) {
@@ -632,7 +637,7 @@ export class StaticFooter extends Footer<StaticFooterProps> {
 
     @computed protected get licenseAndOriginUrlText(): string {
         const { finalUrl, finalUrlText, licenseText, licenseUrl } = this
-        const linkStyle = "fill: #5b5b5b; text-decoration: underline;"
+        const linkStyle = `fill: ${GRAPHER_DARK_TEXT}; text-decoration: underline;`
         const licenseSvg = `<a target="_blank" style="${linkStyle}" href="${licenseUrl}">${licenseText}</a>`
         if (!finalUrlText) return licenseSvg
         const originUrlSvg = `<a target="_blank" style="${linkStyle}" href="${finalUrl}">${finalUrlText}</a>`
@@ -684,7 +689,7 @@ export class StaticFooter extends Footer<StaticFooterProps> {
         const { targetX, targetY } = this.props
 
         return (
-            <g className="SourcesFooter" style={{ fill: "#5b5b5b" }}>
+            <g className="SourcesFooter" style={{ fill: GRAPHER_DARK_TEXT }}>
                 {sources.renderSVG(targetX, targetY)}
                 {note.renderSVG(
                     targetX,

packages/@ourworldindata/grapher/src/header/Header.tsx

@@ -8,7 +8,10 @@ import { computed } from "mobx"
 import { observer } from "mobx-react"
 import { Logo, LogoOption } from "../captionedChart/Logos"
 import { HeaderManager } from "./HeaderManager"
-import { DEFAULT_GRAPHER_FRAME_PADDING } from "../core/GrapherConstants"
+import {
+    DEFAULT_GRAPHER_FRAME_PADDING,
+    GRAPHER_DARK_TEXT,
+} from "../core/GrapherConstants"
 
 @observer
 export class Header extends React.Component<{
@@ -132,18 +135,20 @@ export class Header extends React.Component<{
                     style={{
                         fontFamily:
                             "'Playfair Display', Georgia, 'Times New Roman', 'Liberation Serif', serif",
-                        fontWeight: 500,
                     }}
                     target="_blank"
                     rel="noopener"
                 >
-                    {title.render(x, y, { fill: "#4E4E4E" })}
+                    {title.render(x, y, {
+                        fill: GRAPHER_DARK_TEXT,
+                        fontWeight: 500,
+                    })}
                 </a>
                 {subtitle.renderSVG(
                     x,
                     y + title.height + this.subtitleMarginTop,
                     {
-                        fill: "#4E4E4E",
+                        fill: GRAPHER_DARK_TEXT,
                     }
                 )}
             </g>
@@ -153,6 +158,13 @@ export class Header extends React.Component<{
     private renderTitle(): JSX.Element {
         const { manager } = this
 
+        // avoid linking to a grapher/data page when we're already on it
+        if (manager.isOnCanonicalUrl) {
+            return (
+                <h1 style={this.title.htmlStyle}>{this.title.renderHTML()}</h1>
+            )
+        }
+
         // on smaller screens, make the whole width of the header clickable
         if (manager.isMedium) {
             return (

packages/@ourworldindata/grapher/src/header/HeaderManager.ts

@@ -16,4 +16,5 @@ export interface HeaderManager {
     isMedium?: boolean
     framePaddingHorizontal?: number
     framePaddingVertical?: number
+    isOnCanonicalUrl?: boolean
 }

packages/@ourworldindata/grapher/src/lineCharts/LineChart.tsx

@@ -1184,7 +1184,9 @@ export class LineChart
                         : this.defaultRightPadding
                 )
                 // top padding leaves room for tick labels
-                .padTop(6),
+                .padTop(6)
+                // bottom padding avoids axis labels to be cut off at some resolutions
+                .padBottom(2),
             verticalAxis: this.verticalAxisPart,
             horizontalAxis: this.horizontalAxisPart,
         })

packages/@ourworldindata/grapher/src/modal/Modal.scss

@@ -1,5 +1,9 @@
 $modal-padding: 1.5em;
 
+// the base font size for modals is 16px and on
+// smaller devices scales down to 14px.
+// it is never scaled up though.
+
 .modalOverlay {
     position: absolute;
     // -1px to hide the frame border

packages/@ourworldindata/grapher/src/modal/SourcesModal.tsx

@@ -62,8 +62,9 @@ export class SourcesModal extends React.Component<{
 
     @computed private get modalBounds(): Bounds {
         const maxWidth = 740
-        const padWidth = Math.max(16, (this.tabBounds.width - maxWidth) / 2)
-        return this.tabBounds.padHeight(16).padWidth(padWidth)
+        // using 15px instead of 16px to make sure the modal fully covers the OWID logo in the header
+        const padWidth = Math.max(15, (this.tabBounds.width - maxWidth) / 2)
+        return this.tabBounds.padHeight(15).padWidth(padWidth)
     }
 
     private renderSource(column: CoreColumn): JSX.Element {

packages/@ourworldindata/grapher/src/scatterCharts/ScatterPlotChart.tsx

@@ -505,8 +505,12 @@ export class ScatterPlotChart
     @computed get dualAxis(): DualAxis {
         const { horizontalAxisPart, verticalAxisPart } = this
         return new DualAxis({
-            // top padding leaves room for tick labels
-            bounds: this.bounds.padRight(this.sidebarWidth + 20).padTop(6),
+            bounds: this.bounds
+                .padRight(this.sidebarWidth + 20)
+                // top padding leaves room for tick labels
+                .padTop(6)
+                // bottom padding makes sure the x-axis label doesn't overflow
+                .padBottom(2),
             horizontalAxis: horizontalAxisPart,
             verticalAxis: verticalAxisPart,
         })

packages/@ourworldindata/grapher/src/stackedCharts/AbstractStackedChart.tsx

@@ -172,8 +172,12 @@ export class AbstractStackedChart
             paddingForLegend,
         } = this
         return new DualAxis({
-            // top padding leaves room for tick labels
-            bounds: bounds.padRight(paddingForLegend).padTop(6),
+            bounds: bounds
+                .padRight(paddingForLegend)
+                // top padding leaves room for tick labels
+                .padTop(6)
+                // bottom padding avoids axis labels to be cut off at some resolutions
+                .padBottom(2),
             horizontalAxis: horizontalAxisPart,
             verticalAxis: verticalAxisPart,
         })

packages/@ourworldindata/grapher/src/stackedCharts/MarimekkoChart.tsx

@@ -523,7 +523,7 @@ export class MarimekkoChart
             Math.max(whiteSpaceOnLeft, this.longestLabelWidth) -
             whiteSpaceOnLeft
         return this.bounds
-            .padBottom(this.longestLabelHeight)
+            .padBottom(this.longestLabelHeight + 2)
             .padBottom(labelLinesHeight)
             .padTop(this.legend.height + this.legendPaddingTop)
             .padLeft(marginToEnsureWidestEntityLabelFitsEvenIfAtX0)

packages/@ourworldindata/grapher/src/stackedCharts/StackedDiscreteBarChart.tsx

@@ -164,7 +164,8 @@ export class StackedDiscreteBarChart
     }
 
     @computed private get bounds(): Bounds {
-        return (this.props.bounds ?? DEFAULT_BOUNDS).padRight(10)
+        // bottom padding avoids axis labels to be cut off at some resolutions
+        return (this.props.bounds ?? DEFAULT_BOUNDS).padRight(10).padBottom(2)
     }
 
     @computed private get missingDataStrategy(): MissingDataStrategy {

packages/@ourworldindata/utils/src/SharedConstants.ts

@@ -0,0 +1,2 @@
+export const DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID =
+    "sources-and-processing" as const

packages/@ourworldindata/utils/src/index.ts

@@ -629,3 +629,5 @@ export {
     gdocIdRegex,
     detailOnDemandRegex,
 } from "./GdocsConstants.js"
+
+export { DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID } from "./SharedConstants"

site/DataPageContent.tsx

@@ -8,7 +8,10 @@ import { GrapherWithFallback } from "./GrapherWithFallback.js"
 import { formatAuthors } from "./clientFormatting.js"
 import { ArticleBlocks } from "./gdocs/ArticleBlocks.js"
 import { RelatedCharts } from "./blocks/RelatedCharts.js"
-import { DataPageContentFields } from "@ourworldindata/utils"
+import {
+    DataPageContentFields,
+    DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID,
+} from "@ourworldindata/utils"
 import { AttachmentsContext, DocumentContext } from "./gdocs/OwidGdoc.js"
 import StickyNav from "./blocks/StickyNav.js"
 import cx from "classnames"
@@ -67,7 +70,10 @@ export const DataPageContent = ({
         { text: "Related Data", target: "#related-data" },
         { text: "All Charts", target: "#all-charts" },
         { text: "FAQs", target: "#faqs" },
-        { text: "Sources & Processing", target: "#sources-and-processing" },
+        {
+            text: "Sources & Processing",
+            target: "#" + DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID,
+        },
         { text: "Reuse This Work", target: REUSE_THIS_WORK_ANCHOR },
     ]
 
@@ -401,7 +407,9 @@ export const DataPageContent = ({
                             <div className="section-wrapper grid">
                                 <h2
                                     className="data-sources-processing__title span-cols-2 span-lg-cols-3 col-md-start-2 span-md-cols-10 col-sm-start-1 span-sm-cols-12"
-                                    id="sources-and-processing"
+                                    id={
+                                        DATAPAGE_SOURCES_AND_PROCESSING_SECTION_ID
+                                    }
                                 >
                                     Sources and Processing
                                 </h2>