[Core] Guide Rework Part 2 #7501

Sharrq · 2025-11-02T00:23:38Z

Additional new guide components as part of the larger rework that i have been working on. Alongside this PR, I also split some of the previous components to create some more generalized base components that people can reuse for other purposes if they decide to use them for other purposes or use cases.

Cast Efficiency - I differentiated the layout depending on whether the spell has charges or not. If it does not have charges it will show colored segments for when the ability is on cooldown with white pins for the casts. Gaps are shown as red highlights in the background. For the charged spells, the bullet graph main bar shows the total casts with the target line for the number that was possible. The bar beneath it shows the amount of time that they were capped on charges. I separated SegmentedTimeline and BulletGraph from this to be reused elsewhere. This also shows the difference between the standard layout and the compact layout that consolidates things into a single row.

Intensity Graph - This separates the fight into blocks of equal time depending on the length of the fight and the number of blocks. In this example, we are taking the ignite damage ticks over the fight and creating a heatmap of where the largest spikes of ignite was. I separated Heatmap from this to be used for other use cases.

Intensity Bar - Similar to Intensity Graph, this shows the amount of time ignite was above particular thresholds. StackedBarChart was separated from this to be used elsewhere.

Damage Contribution - This shows the amount of damage that was contributed to some time period. In this case any damage done while Combustion was active. DonutChart was separated from this to be used elsewhere.

Also in this PR I created GuideDataWrapper to consolidate the overall style elements that i have been including with every guide component i have been creating, this just helps to keep it consistent across them. I also moved the relevant Divs from GuideDivs to this since the wrapper can be used wherever in place of using all those style divs repeatedly.

I purposely didnt overwrite CastEfficiencyBar because i didnt want to replace all uses of it in this PR, so this does duplicate that component with some changes. Ill do that replacement in a separate PR assuming people like it. Along with this rework i removed some options like the gap highlight mode and just had it show the gaps regardless.

…into guide-rework-2

emallson · 2025-11-05T23:55:00Z

i haven't forgotten about this PR, its just going to take a bit to get through. rather than forcing you to go back and split it up, i'm going to basically split it up in review, focusing on one or two components at a time to make it manageable. so if you see a review come in and it only covers a couple things, don't be surprised.

Code Review Status

Non-Code Notes

Besides code stuff, I have a few notes about the design & composition.

Colors

Up front: colors are fucking hard. I don't have a silver bullet for this, but what I have found works well is using one highly-saturated color to highlight the thing you want, and then low-saturation colors for non-focus, informative elements.

An examples of what I'd say are not great choices:

Both colors here are low-saturation and low-brightness, which means the whole thing is kind of visually indistinct. One of the colors should be high-saturation / high-brightness so that it stands out. The current state makes the pins the element of focus (because they are bright, though they lack saturation), which is probably not correct.

For an element like this, my preference would be to make the downtime segment the high-vis color so that the eye is drawn to the problem points (this is what is done in the downtime timeline, seen below)

The other way to do it is done for the uptime segment to be bright, as done here:

I have my preference, but I'm open to persuasion that we should highlight uptime rather than downtime here.

Another element that I think does this quite well is this chart:

The high-DPS points are highly saturated, while the low dps points have low saturation. The whole thing has somewhat inconsistent brightness (low dps values have high brightness, high dps values have low brightness), but that is really annoyingly time-consuming to fix because simply adjusting the S value in HSL is not actually going to produce good-looking results. Overall, it looks quite good so having an inverse relation between saturation and brightness might be good tech for later stuff.

All of that said: I don't think we need to let people choose colors for this. Or if we do, it should be from a limited palette (magic schools don't work great as illustrated by the mage specs: everything is fire, or everything is frost, or everything is arcane). Individually, choosing colors can look appealing. However, across many repeated elements it becomes visually chaotic. Compare these:

current:

Arcane (rescaled using hsl(from ${ARCANE} h 15% 20%))

Frost (rescaled using hsl(from ${FROST} h 15% 20%))

These colors are not ideal (the Frost and Arcane colors are kinda defined by being bright & saturated) but IMO the gaps are much clearer. I think letting a spec page have a "theme color" to use (so spec pages look like spec pages) is okay, but varying it per-element is probably not what we want.

As an aside: I would like to make our color usage more consistent and readable, and this nudge to not introduce even more new, different colors is a part of that :)

Role of Charts & Diagrams

I think it is helpful to look at a chart / diagram and ask "why shouldn't this be a table?". Nott literally an HTML <table>, but a structured list of numbers like this:

For every one of these diagrams: why should we use it instead of a table? There are two common reasons:

There is too much data to put in a table. Timelines are a great example of this. They summarize thousands of data points in a compact form. Reading a table with thousands of rows isn't very useful.
They visualize the relation between values in the table (including in ways that are helpful for people that aren't numerically inclined)

Both of these do the latter:

Fundamentally, they're actually the same thing in a different form: they help visualize the ratio between each pair of numbers in the table.

Now, these:

Written as a table:

Spell	Efficiency	Casts	Max Casts	Capped Time
Phoenix Flames	99%	51	52	3s
Fire Blast	99%	138	140	6s

The table is smaller than the charts, so the reason for the diagram can't be "too much data", it has to be comparison.

What do you want to compare? Usually either:

Casts to Max Casts (aka efficiency / downtime) within the same row (aka the same spell), or
Efficiency across rows (aka different spells)

I'd argue that the structure of these diagrams actually encourages comparing casts / max casts with capped time, which is wrong! This is due to these bars being placed next to each other, while simultaneously being visually separated from other cast efficiency diagrams.

IMO the tabular form with embedded bars is both more space efficient and clearer.

It doesn't have the same eye-catching "what is this", but it is more useful. Making things look good is worthwhile, but it shouldn't be at the cost of (much) functionality.

There's a balance here---once of the big bits of feedback we got from a very loud minority after subcreation migrated to Archon was that Archon was too space-inefficient. People couldn't fit nearly as much on one screen. But that spacing had a purpose: it made Archon look good! We made some compromises (eliminating some padding that cost more than it was worth), but ultimately it is a judgment call. my 2c is that the BulletGraph component doesn't pass the bar yet.

Other Notes

Intensity Chart

I think this turned out really well. I have some concerns about comparability across different reports, but within the same report it is awesome. My one nit is that when viewing per target (or any multiple) that these to be laid out in tabular form instead of alternating labels and diagrams:

Here is what the per-target currently looks like

Here is some quick css hackery to make them more heatmap-style:

(yes, they're no longer square. Not sure they need to be square, but also with less-hacky changes (e.g. reducing the bin count slightly) they could be made square)

Intensity Bar

It feels weird that the data is basically repeated twice. Once in the "table" data-strip at the top, and once in the diagram itself:

Segmented Timeline

I like how the pins panned out. They may interfere with zero-padding stacking of these, but having a "start of bar" marker helps solve the problem of "perfectly back-to-back cooldowns looks like one long cooldown"

Sharrq · 2025-11-06T00:30:32Z

Looking through the feedback. I am probably going to make some changes here

I agree that having the labels and boxes side by side instead of alternating on the intensity graph is better, ill change it
Ill eliminate the redundant information on intensitybar
I do like having the bullet graph for the charged spells, but i agree about the minor bar conflicting, so ill probably move the capped time to a stat box alongside the cast efficiency percent and then just have the one bar/target line.
I see the point on the color saturation. I tried a few options originally including one that was a patterned background instead of a solid color (like a gray bar with red diagonals across it), but didnt really like it so i went back to the solid background. Ill mess with the colors and make them uniform throughout.
As for tables, I would argue that tables are often boring and require filtering through a bunch of text to find the info you are looking for (especially when they have a lot of data). At a glance, for something like cast efficiency for charged spells, I can easily see "bar has a lot of empty space and thats bad". Drawing attention to the good/bad first and the numbers secondarily.

emallson

i am not the biggest fan of donut charts---they are very big, and aside from looking nice they don't do much. comparing ratios is easier with a bar chart (or a stacked bar chart, like you have in this PR). I'd pitch deleting the Donut Chart entirely, personally, but not going to force that issue and am fine including it.

However, the DamageContribution component doesn't really have a place (IMO). it doesn't do enough as a helper to justify the extra space. The useful bits of it (mostly the legend) should be moved to the base component.

emallson · 2025-11-06T16:50:15Z

src/interface/guide/components/DonutChart.tsx

+ * @param showCenterText - Whether to show center text with total (default: true)
+ * @param centerText - Custom center text override
+ */
+export default function DonutChart({


what is the purpose of having a new DonutChart (instead of using parser/ui/DonutChart)? is it primarily the new aesthetic parts (e.g. hover animation?)

(it does look nicer than the old one)

emallson · 2025-11-06T17:20:58Z

src/interface/guide/components/DamageContribution.tsx

+ * @param helperText - Optional helper text to display below the header
+ * @param stats - Optional stat cards to display in the header
+ */
+export default function DamageContribution({


i don't believe that this component is at the correct level of abstraction. that is making it very narrow & specific.

this component merges bits of analysis (injected via calculateContribution) with display (Legend layout with DonutChart).

what i'd suggest is:

move the Legend stuff to DonutChart (can be optional with a boolean prop)

maybe add a total prop to DonutChart to make it easy to add the "other" segment (not convinced we need this, since ultimately it is just total - sum(value))

remove this component

based on your use of this component for Combustion, I don't think that it makes sense to have a separate component for DamageContribution. this component has almost no actual logic, and primarily just calls the user-supplied calculateContribution for every element. but: the hard part of using this is not calling calculateContribution, but writing it---and this doesn't help with that.

emallson · 2025-11-06T17:22:25Z

src/interface/guide/components/DamageContribution.tsx

+  // Add "Other" to contributions if it exists
+  const allContributions: Array<SpellContribution & { isOther?: boolean }> = [...contributions];


this copy is not needed, contributions is never used again.

emallson · 2025-11-06T17:24:57Z

src/interface/guide/components/DamageContribution.tsx

+  // Calculate total from all damage (including unspecified)
+  const overallTotal = calculateContribution(-1); // -1 signals to get total of all


i mentioned eliminating calculateContribution above but if it stays, it shouldn't have this -1 behavior. the user can just pass in the overallTotal prop. they would already to have to write a separate branch to handle the -1 case and calculate it, so it isn't saving any work to just pass the value in.

emallson · 2025-11-06T17:31:01Z

src/analysis/retail/mage/fire/guide/Combustion.tsx

+              spells={[
+                { spell: TALENTS.PYROBLAST_TALENT, color: '#ff6600' },
+                { spell: SPELLS.FIRE_BLAST, color: '#ff9933' },
+                { spell: SPELLS.PHOENIX_FLAMES_DAMAGE, color: '#ffcc00' },
+                { spell: SPELLS.IGNITE, color: '#910808ff' },
+                { spell: SPELLS.FLAMESTRIKE, color: '#cc3300' },
+                { spell: SPELLS.ARCANE_PHOENIX_DAMAGE, color: '#7b1d92ff' },
+              ]}
+              calculateContribution={(spellId: number) =>
+                this.combustionDamageTracker.getDamageForSpell(spellId)
+              }


so as an example of the issue with calculateContribution: this could be replaced by:

calling [ ... ].map(entry => ({ ...entry, value: getDamageForSpell(entry.spell) }))

passing totalValue={combustionDamageTracker.totalDamage}

with virtually no additional work on the consumer side, but without the added logic & complexity in the DamageContribution component.

ToppleTheNun · 2025-11-06T17:48:27Z

I haven't gone through this full thing yet but I wanted to echo the donut chart feedback.

Sharrq · 2025-11-06T18:56:54Z

I didn't notice that there was a donut chart already, my mistake. I'll take a look at it and see if it's worth replacing it with mine.

emallson · 2025-11-06T18:58:19Z

I looked a bit as I was reviewing and I think it probably is. I'm not sure it's API compatible, but it probably could be

…

On Thu, Nov 6, 2025, 1:57 PM Sharrq ***@***.***> wrote: *Sharrq* left a comment (WoWAnalyzer/WoWAnalyzer#7501) <#7501 (comment)> I didn't notice that there was a donut chart already, my mistake. I'll take a look at it and see if it's worth replacing it with mine. — Reply to this email directly, view it on GitHub <#7501 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABFOTEXUT7ARCWP2V6CEMUD33OKYZAVCNFSM6AAAAACK4CQJIGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTIOJYHEZTKMRXGI> . You are receiving this because you commented.Message ID: ***@***.***>

Sharrq · 2025-11-07T00:00:05Z

Ok. I havent messed with the Donut Chart yet. But made some other adjustments.

I reskinned the colors for things and picked out some muted spec colors and went that direction

I also carried those colors into IntensityChart and IntensityBar

I cleaned up IntensityBar a little bit and added a legend to it instead of the stat boxes because i didnt like that the damage thresholds were buried in the tooltip

And I reworked the CastEfficiencyRibbon to not show a secondary bar on the bullet graph and to move the capped time into a stat box

Also, I did not change the per target section of IntensityChart largely because It tends to make the boxes on the heatmap incredibly small, especially if the targets have long names or if it is constrained to a smaller container like if it is used alongside one of the explanations. I could have reduced the number of blocks to compensate I suppose, but it started to get really annoying to work with ... like the text of the names needed to be large enough, but the bigger you make the text the less room the heatmap boxes have .... so it seemed better to keep it the way it is.

…into guide-rework-2

src/parser/ui/DonutChart.tsx

emallson

`BulletGraph`

The cast count bars here are CastEfficiencyRibbons, which in turn render BulletGraphs. I think bullet graphs are an interesting potential visualization. We do a lot of "show point value X, vs (often hidden) target values A, B, C...", which bullet graphs cover.

However, I think this implementation needs work---both on the code side and the design side.

There are two distinguishing features for Bullet Graphs:

The use of the background to communicate a (not necessarily linear) scale, like our Bad / Ok / Good values. (implemented as PerformanceZone)
The presence of a target "tick" showing the goal, which the progress bar may exceed. (implemented as TargetMarker)

The implementation as it is in this PR does not have functional PerformanceZones, but this is easy to fix so it isn't a big deal. However, after fixing them they aren't really...visible?

The colors used in CastEfficiencyRibbon are very muted (opacity of ~15% against a black-adjacent background), and the bar itself completely covers the backgrounds. This leads to a situation where if you can see the backgrounds, it looks more like a StackedBar.

A little bit more CSS allows insetting the bar, but tbh I wasn't able to find a combination of colors that I think worked well. That isn't to say we can't, but, well, colors are hard and using the bright primary red/yellow/green doesn't make it easier:

My 2c would be to cut BulletGraph from this PR. It wasn't really being used in the elements you presented (those were really just bars with a max-value line---which is still useful, but also much simpler and could be less code). I think they might be interesting to investigate further, but need some focused work to make good (and maybe some changes to our core color palette 🙃 )

if you are interested, here are the css changes I made for the first "fixed" screenshot:

diff --git a/src/interface/guide/components/BulletGraph.tsx b/src/interface/guide/components/BulletGraph.tsx
index 962ac69e28..04c0872ec2 100644
--- a/src/interface/guide/components/BulletGraph.tsx
+++ b/src/interface/guide/components/BulletGraph.tsx
@@ -104,35 +104,40 @@
 `;
 
 const GraphBackground = styled.div`
+  position: absolute;
+  top: 0;
+  left: 0;
+  right: 0;
+  bottom: 0;
+
   width: 100%;
-  height: 32px;
   background: rgba(0, 0, 0, 0.4);
   border-radius: 4px;
   overflow: hidden;
   display: flex;
-  position: relative;
   box-shadow: inset 0 1px 3px rgba(0, 0, 0, 0.4);
-  border: 1px solid rgba(255, 255, 255, 0.05);
+  z-index: 0;
+  box-sizing: content-box;
 `;
 
 const PerformanceZone = styled.div<{ width: number; color: string }>`
+  background-color: ${(props) => props.color};
   width: ${(props) => props.width}%;
   height: 100%;
   position: relative;
 `;
 
 const MainBar = styled.div<{ width: number; color: string }>`
-  position: absolute;
-  left: 0;
-  top: 0;
   width: ${(props) => props.width}%;
-  height: 32px;
   background: ${(props) => props.color};
   display: flex;
   align-items: center;
   padding-left: 8px;
   transition: width 0.3s ease;
-  border-radius: 4px 0 0 4px;
+  margin-top: 1rem;
+  margin-bottom: 1rem;
+  z-index: 1;
+  box-shadow: 0 0 3px black;
 `;
 
 const BarLabel = styled.span`
@@ -149,7 +154,7 @@
   position: absolute;
   left: 100%;
   top: 0;
-  height: 32px;
+  bottom: 0;
   display: flex;
   align-items: center;
   transform: translateX(-50%);

emallson · 2025-11-08T18:08:07Z

src/interface/guide/components/BulletGraph.tsx

+
+const GraphBackground = styled.div`
+  width: 100%;
+  height: 32px;


avoid using fixed heights for things like this. it means that the resizing the parent element doesn't actually re-size the contents and results in visual breakage.

emallson · 2025-11-08T18:10:24Z

src/interface/guide/components/BulletGraph.tsx

+  position: absolute;
+  left: 0;
+  top: 0;


conventionally, the background would be absolute and the content would be laid out. that isn't always the case because sometimes you need to do it the other way, but in this case I think it makes the most sense.

of note: using position: absolute like this prevents (or at least makes difficult) doing the center-alignment for a bullet graph like shown on wikipedia:

(you can center in absolute with some calc magic but it is harder than display: flex; align-items: center;)

emallson · 2025-11-08T18:11:47Z

src/interface/guide/components/BulletGraph.tsx

+const PerformanceZone = styled.div<{ width: number; color: string }>`
+  width: ${(props) => props.width}%;
+  height: 100%;
+  position: relative;
+`;


this doesn't set the background-color attribute using color, so the PerformanceZones don't actually work.

Suggested change

const PerformanceZone = styled.div<{ width: number; color: string }>`

width: ${(props) => props.width}%;

height: 100%;

position: relative;

`;

const PerformanceZone = styled.div<{ width: number; color: string }>`

width: ${(props) => props.width}%;

background-color: ${(props) => props.color};

height: 100%;

position: relative;

`;

src/parser/ui/DonutChart.tsx

emallson

`CastEfficiencyRibbon` / `CastEfficiencyPanel` / `SegmentedTimeline`

aside from the issues i raised with the BulletGraph last time (which would simplify the charges version of the CastEfficiencyRibbon component, these look good

emallson · 2025-11-13T16:35:33Z

src/interface/guide/components/CastEfficiencyRibbon.tsx

+// Helper functions to convert combat log events to timeline segments/markers
+function createCooldownSegments(
+  spellId: number,
+  events: ReturnType<typeof useEvents>,


nit: this is clearer as AnyEvent[]

Suggested change

events: ReturnType<typeof useEvents>,

events: AnyEvent[],

emallson · 2025-11-13T16:39:39Z

src/interface/guide/components/SegmentedTimeline.tsx

+export default function SegmentedTimeline({
+  windows,
+  segments,
+  markers = EMPTY_MARKERS,


nit: this can just be []

Suggested change

markers = EMPTY_MARKERS,

markers = [],

emallson · 2025-11-13T16:44:31Z

src/interface/guide/components/SegmentedTimeline.tsx

+  /** Optional windows to show on the timeline (defaults to full fight if not specified) */
+  windows?: TimeWindow[];


multiple windows makes this component a lot more complex to get right. however, 90% of that complexity comes from the possibility of overlapping windows.

consider adding a dev-mode error check like:

if (import.meta.env.DEV) { windows.sort((a, b) => a.startTime - b.startTime); windows.forEach((window, index) => { const nextWindow = windows[index + 1]; if (nextWindow && nextWindow.startTime < window.endTime) { throw new Error("must not provide overlapping windows to SegmentedTimeline"); } }); }

alternately, we could just document that in the docblock and if people do it it will look broken :)

emallson · 2025-11-13T16:51:55Z

src/interface/guide/components/CastEfficiencyRibbon.tsx

+    return wastedTime;
+  };
+
+  const wastedTime = calculateWastedTime();


this needs to be memoized with useMemo or every re-render is going to reprocess all events for every CastEfficiencyRibbon rendered.

Suggested change

const wastedTime = calculateWastedTime();

const wastedTime = useMemo(calculateWastedTime(), [eventHistory, hasCharges, maxCharges]);

the function is pretty complicated, so you might want to move it out of the component and call it with proper parameters instead like:

Suggested change

const wastedTime = calculateWastedTime();

const wastedTime = useMemo(() => calculateWastedTime(eventHistory, ability), [ability, eventHistory]);

emallson · 2025-11-13T17:04:26Z

src/interface/guide/components/IntensityBar.tsx

+  return { h: 35, s: 90, l: 55 };
+}
+
+function generateGradient(baseColor: string): string[] {


not getting through a full heatmap review right now, but you can replace the color parsing and gradient construction code with css relative colors like:

function generateGradient(baseColor: string): string[] { return [ // Lightest - tier 1 `hsl(from ${baseColor} h calc(s - 10) calc(l + 20))`, // Lighter - tier 2 `hsl(from ${baseColor} h calc(s - 5) calc(l + 10))`, // Base - tier 3 baseColor, // Darker - tier 4 `hsl(from ${baseColor} calc(h - 10) s calc(l - 10))`, // Darkest - tier 5 `hsl(from ${baseColor} calc(h - 20) s calc(l - 20))`, ]; }

this will allow using colors that aren't written in hsl format (the browser will convert automatically)

emallson · 2025-11-13T17:11:22Z

src/interface/guide/components/IntensityBar.tsx

+      samples.push(dps);
+    }
+  }
+


this is very very quadratic complexity. there are some other places with quadratic complexity in this pr but they mostly involve small numbers so it is fine. this is <target count> * <event count> * <fight duration in seconds>.

unfortunately we don't have good primitives for making slicing like this easy, and it needs to be manually unrolled into a loop. ideally it'd be single-pass, but the caller is providing data broken out by target already so it can be a single pass per target like:

for (const target of targets) { const buckets = new Map(); for (const event of target.events) { const index = Math.floor((event.timestamp - fightStart) / sampleInterval); const currentValue = buckets.get(index) ?? 0; buckets.set(index, currentValue + event.amount); } samples.push(...buckets.values()); }

emallson · 2025-11-13T17:12:00Z

src/interface/guide/components/IntensityBar.tsx

+  }
+
+  // Calculate median for threshold centering
+  const sortedSamples = [...samples].sort((a, b) => a - b);


don't copy samples here

Suggested change

const sortedSamples = [...samples].sort((a, b) => a - b);

const sortedSamples = samples.sort((a, b) => a - b);

emallson

sorry for taking so long, final round of comments for the initial PR contents. Heatmap/Intensity bar stuff looks good visually, but has some duplication issues internally. these things are pretty tied together, so it might make sense for them to be in the same file and sharing the same internals.

Alternately, make the stuff that is duplicated in IntensityChart exported from Heatmap / IntensityBar so avoid duplication.

emallson · 2025-11-15T16:06:07Z

src/interface/guide/components/Heatmap.tsx

+  width: ${({ $size }) => $size}px;
+  height: ${({ $size }) => $size}px;
+  flex-shrink: 0;
+  background-color: ${({ $color }) => $color};


using $prop like this doesn't seem to be officially supported but I don't see the junk props on elements in the inspector so...i guess its fine?

emallson · 2025-11-15T16:08:53Z

src/interface/guide/components/IntensityBar.tsx

+function roundThreshold(value: number): number {
+  if (value > 100000) return Math.round(value / 10000) * 10000;
+  if (value > 50000) return Math.round(value / 5000) * 5000;
+  return Math.round(value / 1000) * 1000;
+}


i guess its intended that each of these is bucketed into values 1/10 of the threshold? (so 100k gets 10k, 50k gets 5k)?

emallson · 2025-11-15T16:11:43Z

src/interface/guide/components/IntensityBar.tsx

+  // Create 5 intensity tiers centered around median
+  const thresholds = [
+    { min: 0, max: Math.max(0, medianRounded - step), label: 'Very Low' },
+    { min: Math.max(0, medianRounded - step), max: medianRounded, label: 'Low' },
+    { min: medianRounded, max: medianRounded + step, label: 'Medium' },
+    { min: medianRounded + step, max: medianRounded + step * 2, label: 'High' },
+    { min: medianRounded + step * 2, max: Infinity, label: 'Very High' },
+  ];


the way these are auto-scaled is reasonable as an informative element. however, these aren't comparable across fights. they don't necessarily need to be, but conventionally our stuff is.

there isn't really a way to make it comparable across reports without picking a somewhat-arbitrary center point for the scale, unfortunately. so this is probably okay, but we'll see what people's feedback is.

emallson · 2025-11-15T16:12:45Z

src/interface/guide/components/IntensityChart.tsx

+function parseColor(color: string): HSL {
+  // Handle HSL format
+  const hslMatch = color.match(/hsl\((\d+),\s*(\d+)%?,\s*(\d+)%?\)/);
+  if (hslMatch) {
+    return {
+      h: parseInt(hslMatch[1]),
+      s: parseInt(hslMatch[2]),
+      l: parseInt(hslMatch[3]),
+    };
+  }
+
+  // Handle hex format
+  const hexMatch = color.match(/^#?([a-f\d]{2})([a-f\d]{2})([a-f\d]{2})$/i);
+  if (hexMatch) {
+    const r = parseInt(hexMatch[1], 16) / 255;
+    const g = parseInt(hexMatch[2], 16) / 255;
+    const b = parseInt(hexMatch[3], 16) / 255;
+
+    const max = Math.max(r, g, b);
+    const min = Math.min(r, g, b);
+    const l = (max + min) / 2;
+
+    if (max === min) {
+      return { h: 0, s: 0, l: Math.round(l * 100) };
+    }
+
+    const d = max - min;
+    const s = l > 0.5 ? d / (2 - max - min) : d / (max + min);
+
+    let h = 0;
+    if (max === r) h = ((g - b) / d + (g < b ? 6 : 0)) / 6;
+    else if (max === g) h = ((b - r) / d + 2) / 6;
+    else h = ((r - g) / d + 4) / 6;
+
+    return {
+      h: Math.round(h * 360),
+      s: Math.round(s * 100),
+      l: Math.round(l * 100),
+    };
+  }
+
+  // Default fallback to fire orange
+  return { h: 35, s: 90, l: 55 };
+}
+
+function generateGradient(baseColor: string): string[] {
+  const { h, s, l } = parseColor(baseColor);
+
+  return [
+    // Lightest - tier 1
+    `hsl(${h}, ${Math.max(0, s - 10)}%, ${Math.min(100, l + 20)}%)`,
+    // Lighter - tier 2
+    `hsl(${h}, ${Math.max(0, s - 5)}%, ${Math.min(100, l + 10)}%)`,
+    // Base - tier 3
+    `hsl(${h}, ${s}%, ${l}%)`,
+    // Darker - tier 4
+    `hsl(${Math.max(0, h - 10)}, ${s}%, ${Math.max(0, l - 10)}%)`,
+    // Darkest - tier 5
+    `hsl(${Math.max(0, h - 20)}, ${s}%, ${Math.max(0, l - 20)}%)`,
+  ];
+}
+
+function roundThreshold(value: number): number {
+  if (value > 100000) return Math.round(value / 10000) * 10000;
+  if (value > 50000) return Math.round(value / 5000) * 5000;
+  return Math.round(value / 1000) * 1000;
+}


this is like, entirely duplicated from Heatmap with minor (incompatible!) changes. it should not be.

emallson · 2025-11-15T16:14:07Z

src/interface/guide/components/IntensityChart.tsx

+  // Calculate stats
+  const allBlocks = heatmapData.flatMap((t) => t.blocks);
+  const nonZero = allBlocks.filter((v) => v > 0);
+  const sorted = [...nonZero].sort((a, b) => a - b);


another needless copy.

emallson · 2025-11-15T16:15:17Z

src/interface/guide/components/IntensityChart.tsx

+    ? (() => {
+        // Group targets by name
+        const grouped = data.reduce(
+          (acc, target) => {
+            if (!acc[target.targetName]) {
+              acc[target.targetName] = [];
+            }
+            acc[target.targetName].push(target);
+            return acc;
+          },
+          {} as Record<string, TargetData[]>,
+        );
+
+        // Build data for each unique target name
+        return Object.entries(grouped).map(([name, targets]) => buildTargetData(targets, name));
+      })()


use an if-else instead of a ternary + IIFE

emallson · 2025-11-15T16:16:08Z

src/interface/guide/components/IntensityChart.tsx

+  const median = sorted[Math.floor(sorted.length / 2)] || maxValue / 2;
+  const medianRounded = roundThreshold(median);
+  const step = roundThreshold(median * 0.4);
+  const thresholds = [
+    0,
+    medianRounded - step,
+    medianRounded,
+    medianRounded + step,
+    medianRounded + step * 2,
+  ];


this logic is also duplicated from IntensityBar

emallson · 2025-11-15T16:18:08Z

src/interface/guide/components/StackedBar.tsx

+    <>
+      <strong>{segment.label}</strong>
+      <br />
+      Value: {segment.value.toFixed(0)}
+      <br />
+      Percentage: {percent.toFixed(1)}%
+    </>


use <div> not <br/>

emallson · 2025-11-15T16:19:38Z

src/interface/guide/components/StackedBar.tsx

+  /** Custom label formatter. Receives segment and percentage, returns label content */
+  labelFormat?: (segment: StackedBarSegment, percent: number) => React.ReactNode;
+  /** Custom tooltip formatter. Receives segment and percentage, returns tooltip content */
+  tooltipFormat?: (segment: StackedBarSegment, percent: number) => React.ReactNode;


pick one method to do labels / tooltips. either they should be supplied on Segments (like you're doing currently), or they should be format props. Not both.

my 2c is to remove these and keep the segment props.

…into guide-rework-2

Sharrq · 2025-11-22T19:11:02Z

Moved this back to draft for now. Going to work through the feedback, but wont be able to test those changes until i get the mage specs re-enabled in Midnight which i dont want to do in this PR.

Sharrq added 5 commits October 29, 2025 21:34

More components

7da1001

Split new components to allow reusing generic components

89cd421

Cleanup and Reworking

6c2dc33

documentation and minor cleanup

dbba57e

Generalize CooldownTimeline further to make it more reusable

5c17627

github-actions bot added Mage core The War Within labels Nov 2, 2025

Sharrq added 3 commits November 1, 2025 20:30

changelog

6291fcb

Merge branch 'midnight' of https://github.com/WoWAnalyzer/WoWAnalyzer …

e7af841

…into guide-rework-2

fix some weird background on bulletgraph

fd58588

github-actions bot added Death Knight Druid Hunter Monk Paladin Priest Rogue Shaman Warlock Warrior Demon Hunter Classic Evoker labels Nov 6, 2025

Sharrq force-pushed the guide-rework-2 branch 2 times, most recently from 56cc5b7 to fd58588 Compare November 6, 2025 02:11

Merge branch 'midnight' into guide-rework-2

7d69e6e

emallson requested changes Nov 6, 2025

View reviewed changes

Sharrq added 2 commits November 6, 2025 19:08

pr feedback

5f442aa

Merge branch 'guide-rework-2' of https://github.com/Sharrq/WoWAnalyzer …

ecc9c00

…into guide-rework-2

emallson reviewed Nov 8, 2025

View reviewed changes

src/parser/ui/DonutChart.tsx Outdated Show resolved Hide resolved

emallson requested changes Nov 8, 2025

View reviewed changes

emallson reviewed Nov 13, 2025

View reviewed changes

emallson requested changes Nov 15, 2025

View reviewed changes

Sharrq added 2 commits November 22, 2025 11:37

Merge branch 'midnight' of https://github.com/WoWAnalyzer/WoWAnalyzer …

7ca3ee8

…into guide-rework-2

Remove new DonutChart component, revert changes to existing DonutChart

af3d50b

github-actions bot added the Midnight label Nov 22, 2025

Sharrq marked this pull request as draft November 22, 2025 19:09

		// Add "Other" to contributions if it exists
		const allContributions: Array<SpellContribution & { isOther?: boolean }> = [...contributions];

		// Calculate total from all damage (including unspecified)
		const overallTotal = calculateContribution(-1); // -1 signals to get total of all

		/** Optional windows to show on the timeline (defaults to full fight if not specified) */
		windows?: TimeWindow[];

	const wastedTime = calculateWastedTime();
	const wastedTime = useMemo(calculateWastedTime(), [eventHistory, hasCharges, maxCharges]);

	const wastedTime = calculateWastedTime();
	const wastedTime = useMemo(() => calculateWastedTime(eventHistory, ability), [ability, eventHistory]);

	const sortedSamples = [...samples].sort((a, b) => a - b);
	const sortedSamples = samples.sort((a, b) => a - b);

[Core] Guide Rework Part 2 #7501

Are you sure you want to change the base?

[Core] Guide Rework Part 2 #7501

Uh oh!

Conversation

Sharrq commented Nov 2, 2025

Uh oh!

emallson commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review Status

Non-Code Notes

Colors

Role of Charts & Diagrams

Other Notes

Uh oh!

Sharrq commented Nov 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

emallson left a comment

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!

ToppleTheNun commented Nov 6, 2025

Uh oh!

Sharrq commented Nov 6, 2025

Uh oh!

emallson commented Nov 6, 2025 via email

Uh oh!

Sharrq commented Nov 7, 2025

Uh oh!

Uh oh!

emallson left a comment

Choose a reason for hiding this comment

BulletGraph

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!

Uh oh!

emallson left a comment

Choose a reason for hiding this comment

CastEfficiencyRibbon / CastEfficiencyPanel / SegmentedTimeline

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!

emallson left a comment

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!

emallson commented Nov 5, 2025 •

edited

Loading

Sharrq commented Nov 6, 2025 •

edited

Loading

`BulletGraph`

`CastEfficiencyRibbon` / `CastEfficiencyPanel` / `SegmentedTimeline`