codeant-ai-for-open-source[bot] commented on PR #36073: URL: https://github.com/apache/superset/pull/36073#issuecomment-3686134381
## 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/36073/files#diff-9d98abb6414f39df54ce4cc8dec63000004add1f1885c0567d10c4707f361a74R61-R71'><strong>Accessibility</strong></a><br>The new visible "Search" label is rendered as plain text and is not programmatically associated with the input. Screen readers and assistive tech won't associate the label with the input control. Also the component doesn't forward the `id` prop into the input, so there is no straightforward way to associate a label with the input from parent code.<br> - [ ] <a href='https://github.com/apache/superset/pull/36073/files#diff-d987d4addee4ccb4cccae5bb4376bdf7915776d6e8631f9a777abfa98c08c9a0R237-R256'><strong>Accessibility Concern</strong></a><br>The visible "Search" label in `SearchInput` is not programmatically associated with the input element. Although the input has an `aria-label`, linking a label with `htmlFor` to the input `id` improves accessibility for screen readers and keyboard users. Consider ensuring visible labels are semantically associated with inputs or use `aria-labelledby`.<br> - [ ] <a href='https://github.com/apache/superset/pull/36073/files#diff-bd919fa26ebe6f116bb11bdeb0fd42d9044597bf0e7066e6a2528ea8ba8c3f4eR569-R576'><strong>Accessibility</strong></a><br>The added "Search by" control (label + SearchSelectDropdown) is not associated with a form label via htmlFor/id or ARIA attributes. That can harm keyboard navigation and screen reader usability. Verify SearchSelectDropdown accepts an id/aria-label and that a proper label element or aria attributes are added.<br> - [ ] <a href='https://github.com/apache/superset/pull/36073/files#diff-9d98abb6414f39df54ce4cc8dec63000004add1f1885c0567d10c4707f361a74R29-R69'><strong>i18n completeness</strong></a><br>The PR adds translation for the static "Search" label but the input placeholder (`"${count} records..."`) remains untranslated. That placeholder includes user-visible text and a numeric count and should be localized (and ideally handle pluralization) for full i18n coverage.<br> - [ ] <a href='https://github.com/apache/superset/pull/36073/files#diff-7805c2c5216a1be40376bce20707d769ba00a5765b6f75ae8898e01dc4e40dd6R44-R45'><strong>Default-selection behavior</strong></a><br>The component forces a fallback empty-string value when no `value` and no `searchOptions` exist. Passing `''` as the Select `value` can produce an unexpected blank selection or prevent placeholder rendering. Prefer using `undefined` (or explicitly computed selectedValue) so the Select can render placeholder or remain uncontrolled when there are no options.<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]
