codeant-ai-for-open-source[bot] commented on PR #36932: URL: https://github.com/apache/superset/pull/36932#issuecomment-3716572520
## Nitpicks 🔍 <table> <tr><td>🔒 <strong>No security issues identified</strong></td></tr> <tr><td>⚡ <strong>Recommended areas for review</strong><br><br> - [ ] <a href='https://github.com/apache/superset/pull/36932/files#diff-449931f1bf449596ce5e3ab49828ee91c5fc3532cf191e4dbd44fff4ba779c61R162-R169'><strong>Theme persistence</strong></a><br>Ensure this new token is included in theme export/import, default theme objects, and any theme editor/schema. If omitted from serialization or defaults, custom selection colors will be lost when saving/loading themes.<br> - [ ] <a href='https://github.com/apache/superset/pull/36932/files#diff-290a501da867d10ce19ea15b193f2445306a8bd07ebc4d7db7773b0c511d4b30R296-R299'><strong>Robust fallback</strong></a><br>The new theme token `colorEditorSelection` is accessed via nullish coalescing to `colorPrimaryBgHover`. Confirm theme typings (SupersetSpecificTokens) include `colorEditorSelection` and consider a final hard-coded fallback to avoid passing `undefined` into CSS if both tokens are missing.<br> - [ ] <a href='https://github.com/apache/superset/pull/36932/files#diff-449931f1bf449596ce5e3ab49828ee91c5fc3532cf191e4dbd44fff4ba779c61R163-R168'><strong>Runtime fallback</strong></a><br>The new field's JSDoc states it "Defaults to colorPrimaryBgHover if not specified", but the type addition alone does not implement this fallback. Every consumer (e.g., AsyncAceEditor) must explicitly use a fallback (nullish-coalescing) to avoid regressions when the token is absent in a theme.<br> - [ ] <a href='https://github.com/apache/superset/pull/36932/files#diff-290a501da867d10ce19ea15b193f2445306a8bd07ebc4d7db7773b0c511d4b30R296-R303'><strong>Selection contrast</strong></a><br>The PR changes only the selection background but does not ensure the selected text color provides sufficient contrast with the new background. Verify selected text remains readable across themes (light/dark and custom `colorEditorSelection` values).<br> - [ ] <a href='https://github.com/apache/superset/pull/36932/files#diff-449931f1bf449596ce5e3ab49828ee91c5fc3532cf191e4dbd44fff4ba779c61R164-R168'><strong>Naming / CSS variable consistency</strong></a><br>Verify the new token's name fits established naming conventions and mapping to generated CSS variables. Confirm downstream code that builds CSS variables or applies tokens handles this property (and its absence) consistently.<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]
