albusshirazi commented on code in PR #8922:
URL: https://github.com/apache/devlake/pull/8922#discussion_r3408172640
##########
config-ui/src/plugins/components/scope-config-select/index.tsx:
##########
@@ -37,14 +37,18 @@ export const ScopeConfigSelect = ({ plugin, connectionId,
scopeConfigId, onCance
const [version, setVersion] = useState(1);
const [trId, setTrId] = useState<ID>();
const [open, setOpen] = useState(false);
-
- const { ready, data } = useRefreshData(() => API.scopeConfig.list(plugin,
connectionId), [version]);
+ const [showAll, setShowAll] = useState(false);
+
+ const { ready, data } = useRefreshData(
+ () => showAll ? API.scopeConfig.listAll(plugin) :
API.scopeConfig.list(plugin, connectionId),
+ [version, showAll],
+ );
const dataSource = useMemo(
() => (data ? (scopeConfigId ? [{ id: 'None', name: 'No Scope Config'
}].concat(data) : data) : []),
- [data, scopeConfigId],
+ [data, scopeConfigId, showAll],
Review Comment:
Looks much better! Moving the `showAll` check into the data fetcher
clarifies the intent perfectly.
Just one minor cleanup detail on line 49:
`[data, scopeConfigId, showAll]`,
You can remove `showAll` from this dependency array. Since `showAll` is no
longer referenced inside the `useMemo` callback on line 48, keeping it in the
array is unnecessary and will flag a React hooks exhaustive-deps linting
warning.
--
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]
