pierrejeambrun commented on code in PR #57680:
URL: https://github.com/apache/airflow/pull/57680#discussion_r2690733978
##########
airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx:
##########
@@ -142,11 +149,32 @@ export const DataTable = <TData,>({
// Default to show columns filter only if there are actually many columns
displayed
const showColumnsFilter = allowFiltering ?? columns.length > 5;
+ const hasModelName = typeof modelName === "string" && modelName.length > 0;
+ const modelNameKey = modelName ?? "";
+ const translateModelName = useCallback(
+ (count: number) =>
+ hasModelName ? translate(modelNameKey, { count }) : translate("items", {
defaultValue: "items" }),
+ [hasModelName, modelNameKey, translate],
+ );
Review Comment:
This can be simplified. `model_nameKey` = modelName ?? "items". (But this
shoudln't really appear)
##########
airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx:
##########
@@ -142,11 +149,32 @@ export const DataTable = <TData,>({
// Default to show columns filter only if there are actually many columns
displayed
const showColumnsFilter = allowFiltering ?? columns.length > 5;
+ const hasModelName = typeof modelName === "string" && modelName.length > 0;
+ const modelNameKey = modelName ?? "";
Review Comment:
Why do we have to handle this case? `modelName` is always specified, it
should probably be converted to a `readonly modelName: string;`. Not sure why
currently it's `readonly modelName?: string;`
##########
airflow-core/src/airflow/ui/src/pages/Connections/Connections.tsx:
##########
@@ -194,9 +194,10 @@ export const Connections = () => {
initialState={tableURLState}
isFetching={isFetching}
isLoading={isLoading}
- modelName={translate("common:admin.Connections")}
+ modelName="admin:connections.connection"
noRowsMessage={<NothingFoundInfo />}
onStateChange={setTableURLState}
+ showRowCountHeading
Review Comment:
Here we pass `showRowCountHeading` and sometimes we don't.
I think by default `showRowCountHeading` should be true and displayed all
the time, i'm not sure there a lot of places where we don't want this. (even
providers, etc.... we want that)
##########
airflow-core/src/airflow/ui/src/components/DataTable/DataTable.tsx:
##########
@@ -142,11 +149,32 @@ export const DataTable = <TData,>({
// Default to show columns filter only if there are actually many columns
displayed
const showColumnsFilter = allowFiltering ?? columns.length > 5;
+ const hasModelName = typeof modelName === "string" && modelName.length > 0;
+ const modelNameKey = modelName ?? "";
+ const translateModelName = useCallback(
+ (count: number) =>
+ hasModelName ? translate(modelNameKey, { count }) : translate("items", {
defaultValue: "items" }),
+ [hasModelName, modelNameKey, translate],
+ );
Review Comment:
Callback is not necessary I think. Especially since we added react-compiler.
(if needed react compiler will add it)
--
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]
