codeant-ai-for-open-source[bot] commented on PR #36780:
URL: https://github.com/apache/superset/pull/36780#issuecomment-3678845986

   ## Nitpicks 🔍
   
   <table>
   <tr><td>🔒&nbsp;<strong>No security issues identified</strong></td></tr>
   <tr><td>⚡&nbsp;<strong>Recommended areas for review</strong><br><br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36780/files#diff-d609e0804191897b0abc0be96b4bc61bfe7d4acc7414e3b18630084cf997d183R270-R287'><strong>Global
 default change</strong></a><br>The PR changes the Table component's global 
defaults to always use a default page size of 50 and only expose "50" in the 
page-size dropdown. This was intended to fix a bug in a specific consumer 
(drill-to-details), but changing the global Table defaults can have unintended 
side effects across many consumers that rely on the previous defaults (default 
15, pageSizeOptions [5,15,25,50,100]). Verify all usages of Table across the 
repo to ensure this change won't break behavior or UX expectations 
elsewhere.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36780/files#diff-d609e0804191897b0abc0be96b4bc61bfe7d4acc7414e3b18630084cf997d183R286-R304'><strong>Pagination
 size consistency</strong></a><br>The component now uses numeric `pageSize` 
state and string `pageSizeOptions`. If a consumer passes custom 
`pageSizeOptions` or `defaultPageSize`, there might be a mismatch where 
`pageSize` is not present in `pageSizeOptions`. This can cause confusing 
pagination UI / unexpected behavior; consider validating/coercing values so the 
visible options and the active page size are always consistent.<br>
   
   - [ ] <a 
href='https://github.com/apache/superset/pull/36780/files#diff-d609e0804191897b0abc0be96b4bc61bfe7d4acc7414e3b18630084cf997d183R270-R304'><strong>Consumer-level
 fix vs. component-level change</strong></a><br>The original bug described is 
specific to drill-to-details. A safer approach is to limit page-size options 
only for that consumer instead of changing the Table component's global 
defaults. Changing the core component increases maintenance surface and may 
hide other pagination issues. Review whether the change should be localized to 
the drill-to-details consumer.<br>
   
   </td></tr>
   </table>
   


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