From 724bad18164dca106135c8fb2ed6d3795cb7ab0c Mon Sep 17 00:00:00 2001
From: Holly Cummins <hcummins@redhat.com>
Date: Wed, 13 Nov 2024 19:50:15 +0000
Subject: [PATCH] Make pie charts render nicely on mobile and be responsive in
 general

---
 src/components/charts/contributions-chart.js | 130 ++++++++++++++-----
 src/templates/extension-detail.js            |  14 +-
 2 files changed, 98 insertions(+), 46 deletions(-)

diff --git a/src/components/charts/contributions-chart.js b/src/components/charts/contributions-chart.js
index 436b69ef5d95..803989b1a19d 100644
--- a/src/components/charts/contributions-chart.js
+++ b/src/components/charts/contributions-chart.js
@@ -3,21 +3,56 @@ import { getPalette } from "../util/styles/style"
 import { LabelList, Legend, Pie, PieChart, ResponsiveContainer, Text, Tooltip } from "recharts"
 import PropTypes from "prop-types"
 import styled from "styled-components"
+import { useMediaQuery } from "react-responsive"
+import { device } from "../util/styles/breakpoints"
 
 const RADIAN = Math.PI / 180
 
+const bigHeight = "520px"
+const mediumHeight = "340px"
+const smallHeight = "260px"
+
+//  We need to set an explicit height for the charts, or the contents don't render at all
+const ChartHolder = styled.div`
+  height: ${bigHeight};
+
+  // noinspection CssUnknownProperty
+  @media ${device.sm} {
+    height: ${mediumHeight};
+  }
+
+  // noinspection CssUnknownProperty
+  @media ${device.xs} {
+    height: ${smallHeight};
+  }
+`
+
+const LegendHolder = styled.div`
+  display: flex;
+  flex-direction: column;
+  justify-content: center;
+  align-items: center;
+`
+
 const LegendSwatch = styled.div`
   height: var(--font-size-10);
   width: var(--font-size-10);
   border-radius: 3px;
   background-color: ${(props) => props.color};
+  border: 0.5px lightgray solid;
 `
 
 const ContributorList = styled.ul`
   overflow: scroll;
-  height: 400px;
   background-color: var(--main-background-color); // this very slightly reduces quite how awful it is if the content overflows to the right-hand side
   padding-inline-start: 0;
+
+  height: calc(${bigHeight} - var(--a-generous-space));
+
+  // noinspection CssUnknownProperty
+  @media ${device.sm} {
+    height: auto;
+  }
 `
 
 const ContributorInformation = styled.li`
@@ -29,7 +64,7 @@ const ContributorInformation = styled.li`
   column-gap: 0.75rem;
   font-size: var(--font-size-10);
   padding: 2px;
-
+  max-width: 200px;
 `
 
 const Contributor = styled.div`
@@ -59,6 +94,9 @@ const ContributionsChart = (props) => {
   const uncolouredContributors = props.contributors
   const uncolouredCompanies = props.companies
 
+  const isMobile = useMediaQuery({ query: device.xs })
+  const isSmallScreen = useMediaQuery({ query: device.sm })
+
   if (uncolouredContributors?.length > 0) {
     const palette = getPalette(uncolouredContributors.length, props.baseColour)
 
@@ -80,40 +118,60 @@ const ContributionsChart = (props) => {
     const contributors = ungroupedContributors.sort((a, b) => a.company === b.company ? (b.contributions - a.contributions) : companyComparator(a.company, b.company))
 
     const lotsOfContributors = contributors.length > 8
+    const shouldRenderLabels = !lotsOfContributors && !isSmallScreen
+    const shouldRenderExternalLegend = isSmallScreen
+
+    // These have to be convincingly smaller than the size of the chartholder, or a ring will go missing
+    const innerRadius = isMobile ? 40 : 70
+    const companyRingWidth = isMobile ? 15 : 25
 
-    const innerRadius = 70
-    const companyRingWidth = 25
+    const width = "100%"
 
     //  we set a blank label if there are a small number of contributors, so we get the line, but we define our own
     // text so we can make it black. the offset in the label list is hand-tuned to put the text near the end of the line
     return (
-      <ResponsiveContainer width={700} height="100%">
-        <PieChart title={"Committers"}
-                  desc={`A pie chart showing that ${contributors[0].name} has made the most commits in the past six months.`}>
-          <Pie data={companies} dataKey="contributions" nameKey="name" innerRadius={innerRadius}
-               outerRadius={innerRadius + companyRingWidth}
-          >
-          </Pie>
-
-          <Pie data={contributors} dataKey="contributions" nameKey="name"
-               innerRadius={innerRadius + companyRingWidth + 5}
-               label={lotsOfContributors ? false : () => ""}
-          >
-
-            {lotsOfContributors ||
-              <LabelList position="outside" offset={21} stroke="none"
-                         fill="var(--main-text-color)"
-                         content={renderCustomizedLabel} valueAccessor={(p) => p} />}
-            }
-          </Pie>
-
-          {lotsOfContributors && <Legend layout="vertical" align="right" verticalAlign="top"
-                                         content={renderLegend} />}
-
-          <Tooltip formatter={((value, name) => [`${value} commits`, name])} />
-
-        </PieChart>
-      </ResponsiveContainer>)
+      <div width={width}>
+        <ChartHolder>
+
+          <ResponsiveContainer width={width} height="100%">
+            <PieChart title={"Committers"}
+                      desc={`A pie chart showing that ${contributors[0].name} has made the most commits in the past six months.`}>
+              <Pie data={companies} dataKey="contributions" nameKey="name" innerRadius={innerRadius}
+                   outerRadius={innerRadius + companyRingWidth}
+              >
+              </Pie>
+
+              <Pie data={contributors} dataKey="contributions" nameKey="name"
+                   innerRadius={innerRadius + companyRingWidth + 5}
+                   label={shouldRenderLabels ? () => "" : false}
+              >
+
+                {shouldRenderLabels &&
+                  <LabelList position="outside" offset={21} stroke="none"
+                             fill="var(--main-text-color)"
+                             content={renderCustomizedLabel} valueAccessor={(p) => p} />}
+                }
+              </Pie>
+
+              {(shouldRenderExternalLegend) || <Legend layout="vertical" align="right" verticalAlign="top"
+                                                       content={renderLegend} />}
+
+              <Tooltip formatter={((value, name) => [`${value} commits`, name])} />
+
+            </PieChart>
+          </ResponsiveContainer>
+        </ChartHolder>
+        {shouldRenderExternalLegend &&
+          renderLegend({
+            payload: contributors.map(entry => {
+              return { payload: { ...entry, value: entry.contributions } }
+            })
+          })
+
+        }
+      </div>)
+
+
   }
 }
 
@@ -150,7 +208,7 @@ const renderLegend = (props) => {
   const { payload } = props
 
   return (
-    <div>
+    <LegendHolder>
       <h5>Commits</h5>
       <ContributorList>
         {
@@ -158,24 +216,24 @@ const renderLegend = (props) => {
             .sort((a, b) =>
               b.payload.contributions - a.payload.contributions
             ).map((entry, index) => {
-            const { payload: { name, value }, color } = entry
+            const { payload: { name, value, fill } } = entry
 
             return (
               <ContributorInformation key={`item-${index}`}>
                 <Contributor>
-                  <LegendSwatch color={color} />
+                  <LegendSwatch color={fill} />
                   <a href={entry?.payload.url}>{name}</a>
                 </Contributor>
                 <span
                   style={{
-                    "text-align": "right"
+                    "textAlign": "right"
                   }}>{value}</span>
               </ContributorInformation>
             )
           })
         }
       </ContributorList>
-    </div>
+    </LegendHolder>
   )
 }
 
diff --git a/src/templates/extension-detail.js b/src/templates/extension-detail.js
index 86d909b55df9..c9d9c435e183 100644
--- a/src/templates/extension-detail.js
+++ b/src/templates/extension-detail.js
@@ -109,6 +109,7 @@ const Metadata = styled.div`
 
 const MainContent = styled.div`
   width: 70%;
+  min-width: 70%;
   display: flex;
   flex-direction: column;
 
@@ -174,11 +175,6 @@ const DocumentationHeading = styled.h2`
   padding-bottom: 10px;
 `
 
-//  I wish this wasn't here, but we need to set an explicit height for the charts, or the contents don't render at all
-const ChartHolder = styled.div`
-  height: 480px; // For now, an arbitrary height, but we should tune
-`
-
 const Logo = ({ extension, isMobile }) => {
   // Size here doesn't matter that much because display size is set in css, but getting smaller makes page load quicker
   return isMobile ?
@@ -368,11 +364,9 @@ const ExtensionDetailTemplate = ({
                         past {numMonthsWithUnit}{" "}
                         (excluding merge commits).</p>)}
 
-                    <ChartHolder>
-                      <ContributionsChart contributors={metadata.sourceControl.contributors}
-                                          companies={metadata.sourceControl.companies} baseColour={"#4695EB"}
-                                          companyColour={"#555"} />
-                    </ChartHolder>
+                    <ContributionsChart contributors={metadata.sourceControl.contributors}
+                                        companies={metadata.sourceControl.companies} baseColour={"#4695EB"}
+                                        companyColour={"#555"} />
 
                     {metadata?.sourceControl?.companies && (
                       <p><i>Company affiliations are derived from GitHub user profiles. Want your company's name to be