sfirke commented on code in PR #45591:
URL: https://github.com/apache/superset/pull/45591#discussion_r4368775818
##########
superset-frontend/src/features/groups/utils.ts:
##########
@@ -18,10 +18,80 @@
*/
import { t } from '@apache-superset/core/translation';
import { SupersetClient } from '@superset-ui/core';
+import {
+ DEFAULT_SORT_COMPARATOR,
+ type LabeledValue,
+ stripSurroundingQuotes,
+} from '@superset-ui/core/components';
import rison from 'rison';
+import {
+ SUBJECT_DETAIL_PROP,
+ SUBJECT_OPTION_FILTER_PROPS,
+ SUBJECT_TEXT_LABEL_PROP,
+ SubjectSelectLabel,
+} from 'src/features/subjects/SubjectSelectLabel';
import { getUserDisplayLabel } from 'src/features/users/utils';
import { FormValues } from './types';
+type UserRecord = { id: number; email?: string } & Parameters<
+ typeof getUserDisplayLabel
+>[0];
+
+/**
+ * Option props AsyncSelect's client-side re-filter should match against for
+ * user options: the display name, the email and the username. These mirror
+ * the columns the server-side `user_all_text` filter searches, so a user the
+ * server matched is never hidden by the picker.
+ */
+export const USER_OPTION_FILTER_PROPS = [
+ ...SUBJECT_OPTION_FILTER_PROPS,
+ 'username',
+];
+
+/**
+ * Build a select option for a user picker.
+ *
+ * The option shows the email under the display name, like the subject-based
+ * owner pickers do, so that a server-side match on email is visible to the
+ * person searching. Plain-text copies of both lines, plus the username, are
+ * attached under the `USER_OPTION_FILTER_PROPS` keys so AsyncSelect's
+ * client-side re-filter can match them (a React label is otherwise filtered
+ * as an empty string), and the display name under `title` so ListView can
+ * serialize the chosen option's label into the URL. The display name is also
+ * the option's `aria-label`, since the virtualized option list only derives
+ * one from a string label and would otherwise announce the numeric user id.
+ * The username is not rendered, so a username match surfaces the user
+ * without explaining why.
+ */
+export const toUserSelectOption = (user: UserRecord) => {
+ const textLabel = getUserDisplayLabel(user);
+ return {
+ value: user.id,
+ label: SubjectSelectLabel({ label: textLabel, secondaryLabel: user.email
}),
+ title: textLabel,
+ 'aria-label': textLabel,
Review Comment:
Good catch, confirmed. The "selected value not in options" fallback in
`AsyncSelect` rebuilt labeled values from only `{ value, label }`, so a
preselected member outside the first page lost its `aria-label`, and also
`title` and the filter props the user pickers rely on for sorting and
typed-token matching. It now spreads the selected value so the rest of the
option survives (2441c81286).
For tests, there's a component-level case in `AsyncSelect.test.tsx`, plus
one each for the Group edit modal and the Role `UsersField` seeded with initial
values. Each seeds a preselected user outside the mocked first page and asserts
an option named by display name exists and none named by id. All three fail
without the fix.
---
🤖 _Drafted by Claude Code, reviewed and approved by @sfirke._
--
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]
