You are viewing a plain text version of this content. The canonical link for it is here.

Posted to notifications@superset.apache.org by "payose (via GitHub)" <gi...@apache.org> on 2025/07/24 12:15:21 UTC

[PR] fix:: preserve correct column order when table layout is changed with time comparison enabled [superset]

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

SUMMARY
This PR fixes a bug where the column names in an aggregated Table chart get displayed in the wrong order after changing the layout, especially when the Time Comparison feature is enabled. The issue was causing column headers to become misaligned or overlapped, leading to confusion when interpreting the data.

Design decisions:
Introduced logic to consistently map metric columns and time comparison columns in the correct order during layout updates.

Ensured the internal data structure reflects the original metric ordering regardless of layout toggles.

BEFORE/AFTER SCREENSHOTS
Before:
<img width="1084" height="403" alt="image" src="https://github.com/user-attachments/assets/e187a613-25be-45cc-aa1f-022530b6b4ca" />

Column names appear in the wrong order or overlapped after layout change.

After:

Column names remain consistent and correctly ordered even after changing the layout.

TESTING INSTRUCTIONS
- [ ] Create a Table chart with aggregated data and multiple metrics.
- [ ] Assign a date filter interval.
- [ ] Enable the Time Comparison feature (e.g., “Previous period” or “1 year ago”).
- [ ] Add the chart to a dashboard.
- [ ] Use the dashboard layout editor to hide/show comparison columns.
- [ ] Verify that the column names remain correctly ordered and are not overlapping.

ADDITIONAL INFORMATION
Has associated issue: Fixes #34134

--
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix:: preserve correct column order when table layout is changed with time comparison enabled [superset]

Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.

korbit-ai[bot] commented on code in PR #34300:
URL: https://github.com/apache/superset/pull/34300#discussion_r2228356702


##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -629,7 +629,7 @@ export default function TableChart<D extends DataRecord = DataRecord>(
       const startPosition = value[0];
       const colSpan = value.length;
       // Retrieve the originalLabel from the first column in this group
-      const originalLabel = columnsMeta[value[0]]?.originalLabel || key;
+      const originalLabel = filteredColumnsMeta[value[0]]?.originalLabel || key;

Review Comment:
   ### Incorrect Group Header Labels with Filtered Columns <sub>![category Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Using filteredColumnsMeta to retrieve the originalLabel could lead to incorrect labels when columns are filtered out due to time comparison settings.
   
   
   ###### Why this matters
   When time comparison is enabled and certain columns are hidden, filteredColumnsMeta won't contain all columns, causing the index value[0] to potentially reference the wrong column or return undefined, leading to incorrect grouping header labels.
   
   ###### Suggested change ∙ *Feature Preview*
   Use the full columnsMeta array to ensure correct label retrieval regardless of filtering:
   ```typescript
   const originalLabel = columnsMeta[value[0]]?.originalLabel || key;
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2cfa9398-eeaa-4f00-ad07-07b071e9b417/upvote) [![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2cfa9398-eeaa-4f00-ad07-07b071e9b417?what_not_true=true)  [![Not in Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2cfa9398-eeaa-4f00-ad07-07b071e9b417?what_out_of_scope=true) [![Not in coding standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2cfa9398-eeaa-4f00-ad07-07b071e9b417?what_not_in_standard=true) [![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2cfa9398-eeaa-4f00-ad07-07b071e9b417)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:7d17ea87-851e-4147-8c52-2e6b367eba40 -->
   
   
   [](7d17ea87-851e-4147-8c52-2e6b367eba40)



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: preserve correct column order when table layout is changed with time comparison enabled [superset]

Posted by "geido (via GitHub)" <gi...@apache.org>.

geido merged PR #34300:
URL: https://github.com/apache/superset/pull/34300


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix:: preserve correct column order when table layout is changed with time comparison enabled [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34300:
URL: https://github.com/apache/superset/pull/34300#issuecomment-3113446348

   @msyavuz Ephemeral environment spinning up at http://35.161.47.46:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix: preserve correct column order when table layout is changed with time comparison enabled [superset]

Posted by "msyavuz (via GitHub)" <gi...@apache.org>.

msyavuz commented on code in PR #34300:
URL: https://github.com/apache/superset/pull/34300#discussion_r2229869443


##########
superset-frontend/plugins/plugin-chart-table/src/TableChart.tsx:
##########
@@ -629,7 +629,11 @@ export default function TableChart<D extends DataRecord = DataRecord>(
       const startPosition = value[0];
       const colSpan = value.length;
       // Retrieve the originalLabel from the first column in this group
-      const originalLabel = columnsMeta[value[0]]?.originalLabel || key;
+      const firstColumnInGroup = filteredColumnsMeta[value[0]];

Review Comment:
   Can't we use startPosition here?
   ```suggestion
         const firstColumnInGroup = filteredColumnsMeta[startPosition];
   ```



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org

Re: [PR] fix:: preserve correct column order when table layout is changed with time comparison enabled [superset]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.

github-actions[bot] commented on PR #34300:
URL: https://github.com/apache/superset/pull/34300#issuecomment-3113415125

   @msyavuz Processing your ephemeral environment request [here](https://github.com/apache/superset/actions/runs/16497762246). Action: **up**. More information on [how to use or configure ephemeral environments](https://superset.apache.org/docs/contributing/howtos/#github-ephemeral-environments)


-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org