codeant-ai-for-open-source[bot] commented on PR #37058: URL: https://github.com/apache/superset/pull/37058#issuecomment-3738913014
## 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/37058/files#diff-3164b6297cf5e9b240931f20f3f5eee3357af4b4393704774cb49ee78cc1b958R31-R34'><strong>Missing runtime default</strong></a><br>The new `textSize` prop comment says it "Defaults to the value of `size` if not provided", but this is only a type-level comment. Verify the EmptyState component implementation actually applies that default at runtime (e.g., `textSize = size` when destructuring props) so consumers get the expected visual behavior.<br> - [ ] <a href='https://github.com/apache/superset/pull/37058/files#diff-ce10adce200200fc1b5fb410f2ab5074e4082b18347037fbac38e2d7b788e48bR120-R136'><strong>Missing imageMap fallback</strong></a><br>When a string `image` prop is provided but is not present in `imageMap`, `mappedImage` becomes `undefined` and the `Empty` component receives an undefined image. This can lead to unexpected UI or missing visuals. Provide a safe fallback (e.g., use `empty.svg`) or render a default icon when map lookup fails.<br> - [ ] <a href='https://github.com/apache/superset/pull/37058/files#diff-bcafda45434bdc4d4dc212a0de479a83bfd2d6348ea4d0c162ff01c9e8b01954R96-R98'><strong>Prop compatibility</strong></a><br>The new props `size="medium"` and `textSize="large"` are passed to `StyledEmptyState` (which wraps `EmptyState`). Verify that `EmptyState` actually accepts these prop names and these string values in its TypeScript definitions / runtime API. If the prop names or allowed values differ, this will be a type/runtime mismatch that could silently no-op or cause unexpected rendering.<br> - [ ] <a href='https://github.com/apache/superset/pull/37058/files#diff-ce10adce200200fc1b5fb410f2ab5074e4082b18347037fbac38e2d7b788e48bR163-R169'><strong>Image / Text size mismatch</strong></a><br>The rendered image size is derived from the original `size` prop while the container max-width and text size can be governed by the new `textSize` (via `effectiveTextSize`) and `containerSize`. If `textSize` is larger than `size` the image will remain small and the visual layout will be inconsistent. Consider making the image sizing use the computed `containerSize` (or `effectiveTextSize`) so image, title, and description scale together.<br> - [ ] <a href='https://github.com/apache/superset/pull/37058/files#diff-3164b6297cf5e9b240931f20f3f5eee3357af4b4393704774cb49ee78cc1b958R31-R34'><strong>Consumer compatibility</strong></a><br>Adding `textSize` changes how consumers may need to control text vs image sizing. Audit all components that render `EmptyState` (and any wrappers) to ensure they pass `textSize` where needed or rely on the intended default. Missing updates could lead to inconsistent UI between image and text sizes.<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]
