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

Grapher redesign: Bug fixes

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

@@ -177,7 +177,12 @@ export class CaptionedChart extends React.Component<CaptionedChartProps> {
     }
 
     @computed protected get bounds(): Bounds {
-        return this.props.bounds ?? this.manager.tabBounds ?? DEFAULT_BOUNDS
+        return (
+            this.props.bounds ??
+            // right padding prevents grapher's frame to be obscured by the chart/map
+            this.manager.tabBounds?.padRight(2) ??
+            DEFAULT_BOUNDS
+        )
     }
 
     @computed protected get boundsForChartArea(): Bounds {

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

@@ -12,6 +12,7 @@ export interface ContentSwitchersManager {
     tab?: GrapherTabOption
     isNarrow?: boolean
     type?: ChartTypeName
+    isLineChartThatTurnedIntoDiscreteBar?: boolean
 }
 
 @observer
@@ -41,7 +42,9 @@ export class ContentSwitchers extends React.Component<{
             case GrapherTabOption.map:
                 return <FontAwesomeIcon icon={faEarthAmericas} />
             case GrapherTabOption.chart:
-                return chartIcons[this.chartType]
+                return this.manager.isLineChartThatTurnedIntoDiscreteBar
+                    ? chartIcons[ChartTypeName.DiscreteBar]
+                    : chartIcons[this.chartType]
         }
     }
 

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

@@ -53,6 +53,7 @@ export interface SettingsMenuManager
         FacetStrategySelectionManager {
     base?: React.RefObject<SVGGElement | HTMLDivElement> // the root grapher element
     showConfigurationDrawer?: boolean
+    isInIFrame?: boolean
 
     // ArchieML directives
     hideFacetControl?: boolean
@@ -265,6 +266,8 @@ export class SettingsMenu extends React.Component<{
     }
 
     @computed get drawer(): Element | null {
+        // use a drop-down menu when embedded in an iframe
+        if (this.manager.isInIFrame) return null
         // use the drawer `<nav>` element if it exists, otherwise render into a drop-down menu
         return this.manager.base?.current?.closest(".related-charts")
             ? null // also use a drop-down menu within the Related Charts section

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

@@ -2231,11 +2231,12 @@ export class Grapher
 
     @action.bound dismissFullScreen(): void {
         // if a modal is open, dismiss it instead of exiting full-screen mode
-        if (this.isModalOpen) {
+        if (this.isModalOpen || this.isShareMenuActive) {
             this.isSelectingData = false
             this.isSourcesModalOpen = false
             this.isEmbedModalOpen = false
             this.isDownloadModalOpen = false
+            this.isShareMenuActive = false
         } else {
             this.isInFullScreenMode = false
         }
@@ -2683,7 +2684,7 @@ export class Grapher
     }
 
     @computed get isOnCanonicalUrl(): boolean {
-        if (!this.canonicalUrl) return false
+        if (!this.canonicalUrl || this.isInIFrame) return false
         return (
             getWindowUrl().pathname === Url.fromURL(this.canonicalUrl).pathname
         )

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

@@ -110,8 +110,7 @@ $zindex-controls-drawer: 140;
     container-type: size;
     container-name: grapher;
 
-    // use box-shadow instead of border to make sure the border is in the foreground
-    box-shadow: 0 0 0 1px $frame-color; // equivalent to `border: 1px solid $frame-color`
+    border: 1px solid $frame-color;
     z-index: $zindex-chart;
 
     * {
@@ -170,7 +169,7 @@ $zindex-controls-drawer: 140;
 // and the left and right borders are hidden. the top and bottom borders are visible,
 // but stretch across the entire page.
 .GrapherComponent.optimizeForHorizontalSpace {
-    box-shadow: none;
+    border: 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)

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

@@ -94,19 +94,6 @@ export class DataTable extends React.Component<{
     manager?: DataTableManager
     bounds?: Bounds
 }> {
-    transformTable(table: OwidTable): OwidTable {
-        if (this.manager.showSelectionOnlyInDataTable) {
-            table = table.filterByEntityNames(
-                this.selectionArray.selectedEntityNames
-            )
-        }
-        return table
-    }
-
-    @computed get transformedTable(): OwidTable {
-        return this.transformTable(this.manager.table)
-    }
-
     @observable private storedState: DataTableState = {
         sort: DEFAULT_SORT_STATE,
     }
@@ -144,11 +131,13 @@ export class DataTable extends React.Component<{
     }
 
     @computed get table(): OwidTable {
-        return this.transformedTable
-    }
-
-    @computed get inputTable(): OwidTable {
-        return this.manager.table
+        let table = this.manager.table
+        if (this.manager.showSelectionOnlyInDataTable) {
+            table = table.filterByEntityNames(
+                this.selectionArray.selectedEntityNames
+            )
+        }
+        return table
     }
 
     @computed get manager(): DataTableManager {
@@ -226,7 +215,7 @@ export class DataTable extends React.Component<{
         // then we prefer "Country/area" over "Country or region"
         // (note that we honour the entity type provided by the author if it is given)
         if (
-            this.entityType !== DEFAULT_GRAPHER_ENTITY_TYPE &&
+            this.entityType === DEFAULT_GRAPHER_ENTITY_TYPE &&
             this.showTitleForAggregateRows
         )
             return "Country/area"

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

@@ -13,7 +13,6 @@
 
     li {
         list-style-type: none;
-        word-break: break-word;
     }
 
     .searchBar {