aminghadersohi commented on code in PR #44894:
URL: https://github.com/apache/superset/pull/44894#discussion_r4260665279
##########
superset-frontend/packages/superset-ui-core/src/components/Select/AsyncSelect.stories.tsx:
##########
@@ -318,3 +318,51 @@ AsynchronousSelect.argTypes = {
},
},
};
+
+const createMockUserFetcher =
+ (responseTime = 0.3) =>
+ (
+ search: string,
+ page: number,
+ pageSize: number,
+ ): Promise<SelectOptionsTypePage> => {
+ const username = search.trim().toLowerCase();
+ const all = USERS.filter(
+ u => !username || u.toLowerCase().includes(username),
+ );
+ const totalCount = all.length;
+ const start = page * pageSize;
+ const deleteCount =
+ start + pageSize < totalCount ? pageSize : totalCount - start;
+ const pageData = all.slice(start, start + deleteCount).map(u => ({
+ label: u,
+ value: u,
+ }));
+ return new Promise(resolve => {
+ setTimeout(() => {
+ resolve({ data: pageData, totalCount });
+ }, responseTime * 1000);
+ });
+ };
+
+/**
+ * Demonstrates allowNewOptions in AsyncSelect. Optimistic custom options
appear
+ * with a [new] badge and are automatically dropped if the server returns a
matching
+ * option upon search completion.
+ */
+export const WithAllowNewOptions = (args: AsyncSelectProps) => (
+ <div style={{ width: DEFAULT_WIDTH }}>
+ <AsyncSelect
Review Comment:
tsc TS2322 here: the spread `args` carry `ref: Ref<RefSelectProps>` from
BaseSelectProps, but AsyncSelect's forwardRef needs `AsyncSelectRef`
(RefSelectProps & { clearCache }). Typing the story args as
`Omit<AsyncSelectProps, 'ref'>` clears it.
##########
superset-frontend/packages/superset-ui-core/src/components/Select/types.ts:
##########
@@ -29,9 +29,16 @@ export type RawValue = string | number;
export type V = string | number | null | undefined;
-export type LabeledValue = AntdLabeledValue;
+export type LabeledValue = AntdLabeledValue & {
Review Comment:
Measured: reverting only this widening drops
SelectAsyncControl/index.tsx(80,7) TS2322 - a file this PR doesn't touch -
while AsyncSelect.test.tsx:513 and Select.test.tsx:612 still fail, since
`value` flows through antd's own LabeledValue. It breaks a consumer without
fixing either test.
##########
superset-frontend/packages/superset-ui-core/src/components/Select/utils.tsx:
##########
@@ -54,23 +58,47 @@ export function isEqual(
// When comparing the values we use the equality
// operator to automatically convert different types
// eslint-disable-next-line eqeqeq
- return actualA == actualB;
+ if (actualA == actualB) {
+ return true;
+ }
+ if (typeof actualA === 'string' && typeof actualB === 'string') {
+ return actualA.trim() === actualB.trim();
Review Comment:
Base-vs-head differential: isEqual(' Sales ','Sales') and
hasOption('Sales',[' Sales ']) both flip false -> true, so two options whose
labels differ only by surrounding whitespace now dedupe to one. isEqual('',' ')
flips too. String/number coercion and case-sensitivity are unchanged.
--
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]
