tbonelee commented on PR #5313:
URL: https://github.com/apache/zeppelin/pull/5313#issuecomment-5012028856

   Thanks for this, and to be clear up front: this PR faithfully implements the 
issue as I wrote it, so the gap is in my issue description, not your work. I've 
added a correction on ZEPPELIN-6471.
   
   The one thing to fix here: the description (following the issue) says the 
table array "does not drive any display," but it does. `aggregations` is 
rendered as a dropdown in `table-visualization.component.html:112`:
   
   ```html
   @for (aggregation of aggregations; track aggregation) {
     <li nz-menu-item ...>{{ aggregation | titlecase }}</li>
   }
   ```
   
   So adopting the pivot's order as canonical keeps the pivot dropdown 
unchanged but reorders the table column's Aggregation dropdown: `Count, Sum, 
Min, Max, Avg` becomes `Sum, Count, Avg, Min, Max`. It is cosmetic, not a 
functional bug.
   
   Since the issue's goal was no unintended visible change, the simplest way to 
get there is to share the type only and let each component keep its own order 
array (typed `readonly AggregationType[]`). That drops the untyped `string[]`, 
keeps the table's `switch` exhaustive, and leaves both dropdowns exactly as 
they are today. Could you also update the description's "no visible impact" 
line and extend the manual check to the table dropdown?


-- 
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]

Reply via email to