villebro commented on a change in pull request #16445:
URL: https://github.com/apache/superset/pull/16445#discussion_r711900081



##########
File path: superset-frontend/src/explore/components/PropertiesModal/index.tsx
##########
@@ -237,16 +231,15 @@ export default function PropertiesModal({
             </FormItem>
             <h3 style={{ marginTop: '1em' }}>{t('Access')}</h3>
             <FormItem label={t('Owners')}>
-              <AsyncSelect
-                isMulti
+              <Select
+                ariaLabel={t('Owners')}

Review comment:
       To avoid the `FormItem` `label` and this `ariaLabel` diverging, could be 
nice to break this out into a `const`.

##########
File path: superset-frontend/src/explore/components/PropertiesModal/index.tsx
##########
@@ -54,7 +49,7 @@ export default function PropertiesModal({
   const [cacheTimeout, setCacheTimeout] = useState(
     slice.cache_timeout != null ? slice.cache_timeout : '',
   );
-  const [owners, setOwners] = useState<OptionsType<OwnerOption> | null>(null);
+  const [owners, setOwners] = useState<SelectValue | null>(null);

Review comment:
       > Should we call this `selectedOwners`? At first, I thought it was 
conflicting with `loadOptions`.
   
   That's a really good idea 👍  this would make it more explicit and less 
likely for other developers misunderstanding this piece of state.




-- 
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]

Reply via email to