alexandrusoare opened a new pull request, #38473:
URL: https://github.com/apache/superset/pull/38473

   ### SUMMARY
   When using the "Match time shift color with original series" checkbox in 
Time Series charts, the color matching only worked for Line Charts. For Bar 
Charts and Area Charts, the time-shifted series (e.g., "1 year ago") would 
display a different color than the original series, even with the checkbox 
enabled.
   
   #### Root Cause
   
   The `getOriginalSeries()` function in `timeOffset.ts` was missing one of the 
three time offset naming patterns used by the backend.
   
   When the "Match time shift color with original series" checkbox is enabled, 
both the original series (e.g., `"COUNT(*)"`) and the derived series (e.g., 
`"COUNT(*), 1 year ago"`) need to produce the **same color key** so they get 
assigned the same color.
   
   The `getOriginalSeries()` function strips the time offset suffix to produce 
this key. However, it only handled two of the three patterns:
   
   | Pattern | Example | Handled? |
   | --- | --- | --- |
   | `<offset>,` | `"1 year ago, groupby_value"` | ✅ Yes |
   | `__<offset>` | `"sum__1 year ago"` | ✅ Yes |
   | `, <offset>` | `"COUNT(*), 1 year ago"` | ❌ **No** |
   
   The third pattern (`, <offset>`) is used when there's a single metric with 
no groupby dimensions - which is a common case for bar/area charts. Since this 
pattern wasn't being stripped, the derived series kept its full name as the 
color key, resulting in a different color than the original series.
   
   #### Solution
   
   Add the missing `, <offset>` pattern to `getOriginalSeries()` and 
`getTimeOffset()` in `timeOffset.ts`:
   
   ```
   // In getOriginalSeries()
   result = result.replace(`, ${compare}`, '');
   
   // In getTimeOffset()
   series.name.includes(`, ${timeOffset}`)
   ```
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   
   #### BEFORE
   
   
https://github.com/user-attachments/assets/ba6612d4-73ee-48a0-ab79-8bbaa1b21515
   
   #### AFTER
   
   
https://github.com/user-attachments/assets/abd68029-11a5-4c37-8c1f-753ddd85212e
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] Required feature flags:
   - [ ] Changes UI
   - [ ] Includes DB Migration (follow approval process in 
[SIP-59](https://github.com/apache/superset/issues/13351))
     - [ ] Migration is atomic, supports rollback & is backwards-compatible
     - [ ] Confirm DB migration upgrade and downgrade tested
     - [ ] Runtime estimates and downtime expectations provided
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to