Fix(Webapp): Apply All4Trees Feedback by arnaudfnr · Pull Request #151 · dataforgoodfr/14_Data4Trees

arnaudfnr · 2026-05-03T21:42:24Z

Objective

Apply All4Trees Feedback on the webapp

Context

Catahlijne from All4Trees reviewed the website and gave some feedback that is available here:
https://outline.services.dataforgood.fr/doc/remarques-sur-la-ui-QBK9SKykJA

The webapp is online at this link: https://test-data4trees.services.d4g.fr/

david-bretaud-dev · 2026-05-04T21:38:24Z

@@ -1,5 +1,5 @@
 asgiref==3.8.1
-coordo @ git+https://github.com/dataforgoodfr/Coordonnees.git@0.3.0#subdirectory=coordo-py
+coordo @ git+https://github.com/dataforgoodfr/Coordonnees.git@arnaudfnr/fix-null-values-in-formulas#subdirectory=coordo-py


Do you need an approval to merge this branch ?

david-bretaud-dev · 2026-05-04T21:52:53Z

+            {chartConfig[payload[0].name]?.label || payload[0].name}:{" "}
+            {payload[0].value} {unit}


A bit prone to bad formating and space issue. I suggest

const name = chartConfig[payload[0].name]?.label || payload[0].name; const tooltip = `${name}: ${payload[0].value}${unit}`;

david-bretaud-dev · 2026-05-04T21:55:40Z

+      <polyline
+        fill="none"
+        points={`${lineStartX},${lineStartY} ${lineMidX},${lineMidY} ${labelX},${labelY}`}


Is it this arrow that routes to the label ? x)

david-bretaud-dev · 2026-05-04T21:58:21Z


  return (
-    <div className="flex flex-row items-center justify-between gap-sm flex-1">
+    <div className="flex flex-row items-center justify-between gap-sm flex-1 mt-5 text-sm">


Better to enforce standard spacing

Suggested change

<div className="flex flex-row items-center justify-between gap-sm flex-1 mt-5 text-sm">

<div className="flex flex-row items-center justify-between gap-sm flex-1 mt-sm text-sm">

david-bretaud-dev · 2026-05-04T22:02:09Z

  iconStart?: ReactNode;
 }>;

+const isChartElement = (node: ReactNode): boolean => {


Happy to have an explanation of all this stuff with isChartElement , flattenChildren, const [valueChildren, chartChildren]...

It's hard to understand and read, and thus, hard to maintain. What's the goal behind ?

The goal behind this is to clearly separate charts from other indicators, to make those other indicators more visible in the popup. Therefore I decided to put all the other indicators of a section inside a card component , to highlight them a bit more.
In order to achieve this, I needed to find a way to recognize the chart components. I created the "ChartComponent" which is and should be used by any component in the App, such as Shadcn's chart container. It encapsulates any chart into a Card component. However I did not find a suitable / elegant way to recignize the presence of this component in the chart's children, so I checked the presence of the string "Chart" in the components display names... This is not a very robust solution.

david-bretaud-dev · 2026-05-04T22:04:10Z

+import {
+  ICON_SIZE,
+  ICON_SIZE_HEADER,
+} from "../../indicators/components/constants";


Suggested change

} from "../../indicators/components/constants";

} from "@features/indicators/components/constants";

david-bretaud-dev · 2026-05-04T22:06:51Z

+        <AlertDescription className={cn("text-muted-foreground text-sm")}>
+          <div>
+            {subtitle && (
+              <>
+                <span>{subtitle}</span>
+                <br />
+              </>
+            )}
+            {date && (
+              <div className="flex flex-row items-center gap-xs text-muted-foreground">
+                <Calendar size={ICON_SIZE} />
+                <p className="pt-0.5">{date}</p>
+              </div>
+            )}
+          </div>
        </AlertDescription>


It can be simplified just by adding a gap when both elements are defined

Suggested change

<AlertDescription className={cn("text-muted-foreground text-sm")}>

<div>

{subtitle && (

<>

<span>{subtitle}</span>

<br />

</>

)}

{date && (

<div className="flex flex-row items-center gap-xs text-muted-foreground">

<Calendar size={ICON_SIZE} />

<p className="pt-0.5">{date}</p>

</div>

)}

</div>

</AlertDescription>

<AlertDescription className={cn("text-muted-foreground text-sm", { "flex flex-col gap-sm": subtitle && data })}>

{subtitle && (

<span>{subtitle}</span>

)}

{date && (

<div className="flex flex-row items-center gap-xs text-muted-foreground">

<Calendar size={ICON_SIZE} />

<p className="pt-0.5">{date}</p>

</div>

)}

</AlertDescription>

david-bretaud-dev · 2026-05-04T22:14:48Z

    root.render(
      <Element
-        className="w-120 max-h-120"
+        className="w-130 max-w-130 max-h-150"


It's starting to not fit the screen for small computers :/ (mine for instance, I need to zoom at 80% my browser to see the full popup)

Ideally, we should compute the height based on the size of the div that contains the map.

For instance, we can provide the containerHeight, compute the maxSize we want (I would remove 12 pixels for visual "padding" in the div), provide a custom tailwind class

getRenderPopupLayer(Element: ..., containerHeight: number) { const maxSizePixel = min(600, containerHeight-12); ... <Element style={{ "--popup-max-height": min(600, containerHeight) }} className="max-h-[--popup-max-height]" ... }

david-bretaud-dev · 2026-05-04T22:15:37Z

        "class-variance-authority": "^0.7.1",
        "clsx": "^2.1.1",
-        "coordo": "github:dataforgoodfr/Coordonnees#0.3.0",
+        "coordo": "github:dataforgoodfr/Coordonnees#arnaudfnr/fix-null-values-in-formulas",


To update when merged

david-bretaud-dev · 2026-05-04T22:16:08Z

+
+export const markChartComponent = <P extends object>(
+  component: ComponentType<P>,
+): ComponentType<P> & { isChartComponent: boolean } =>


Seemed to be not used

david-bretaud-dev

Globally okay with changes, some improvements suggestion for popup display or code cleaning

Blocking part on my side is this react children manipulation 🤔 need alignment

arnaudfnr · 2026-05-07T08:43:56Z

Todo :

Clean code in indicator-section => TBR
make popup size responsive
update to coordo 0.3.1 (when PR on coordo side is merged)
fix popup border style
(BONUS): improve coloring of plotly's taxon chart

arnaudfnr added 4 commits May 1, 2026 00:41

Fix(UI): All4Trees Feedback

f0de1f8

Continue applying feedback

65cec75

Improve pie & bar charts rendering

5630802

Last fixes

c054e6b

arnaudfnr linked an issue May 3, 2026 that may be closed by this pull request

Fix(Webapp): Apply All4Trees feedback to the Webapp #152

Open

Update Coordo and some fake indicators

c791826

david-bretaud-dev reviewed May 4, 2026

View reviewed changes

david-bretaud-dev requested changes May 4, 2026

View reviewed changes

arnaudfnr added 2 commits May 6, 2026 15:08

Fix all4trees & David reviews part 1

16ae571

increase icon size

c9122ed

Improve indicator-section readability, mark chart components

2cc7ef8

		{chartConfig[payload[0].name]?.label \|\| payload[0].name}:{" "}
		{payload[0].value} {unit}

	<div className="flex flex-row items-center justify-between gap-sm flex-1 mt-5 text-sm">
	<div className="flex flex-row items-center justify-between gap-sm flex-1 mt-sm text-sm">

	} from "../../indicators/components/constants";
	} from "@features/indicators/components/constants";

Conversation

arnaudfnr commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Context

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

david-bretaud-dev left a comment

Choose a reason for hiding this comment

Uh oh!

arnaudfnr commented May 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

arnaudfnr commented May 3, 2026 •

edited

Loading

arnaudfnr commented May 7, 2026 •

edited

Loading