Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add meta text to DataView #2348

Merged
merged 3 commits into from
Sep 19, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 81 additions & 49 deletions packages/odyssey-react-mui/src/labs/DataView/DataView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,15 @@ import { DataFilters } from "../DataFilters";
import { EmptyState } from "../../EmptyState";
import { fetchData } from "./fetchData";
import { LayoutSwitcher } from "./LayoutSwitcher";
import { MenuButton } from "../..";
import { MenuButton } from "../../MenuButton";
import { MoreIcon } from "../../icons.generated";
import { TableSettings } from "./TableSettings";
import { Pagination, usePagination } from "../../Pagination";
import { TableLayoutContent } from "./TableLayoutContent";
import { CardLayoutContent } from "./CardLayoutContent";
import { useFilterConversion } from "./useFilterConversion";
import { useRowReordering } from "../../DataTable/useRowReordering";
import { Typography } from "../../Typography";
import {
DesignTokens,
useOdysseyDesignTokens,
Expand All @@ -69,6 +70,20 @@ const AdditionalActionsContainer = styled("div")(() => ({
justifyContent: "flex-end",
}));

const AdditionalActionsInner = styled("div", {
shouldForwardProp: (prop) => prop !== "odysseyDesignTokens",
})<{ odysseyDesignTokens: DesignTokens }>(({ odysseyDesignTokens }) => ({
display: "flex",
alignItems: "center",
gap: odysseyDesignTokens.Spacing2,
}));

const MetaTextContainer = styled("div", {
shouldForwardProp: (prop) => prop !== "odysseyDesignTokens",
})<{ odysseyDesignTokens: DesignTokens }>(({ odysseyDesignTokens }) => ({
marginInlineEnd: odysseyDesignTokens.Spacing2,
}));

const DataView = ({
additionalActionButton,
additionalActionMenuItems,
Expand All @@ -92,6 +107,7 @@ const DataView = ({
isNoResults: isNoResultsProp,
isPaginationMoreDisabled,
isRowReorderingDisabled,
metaText,
noResultsPlaceholder,
onChangeRowSelection,
onReorderRows,
Expand Down Expand Up @@ -262,49 +278,62 @@ const DataView = ({
return;
}, [noResultsPlaceholder, t, isEmpty, isNoResults, emptyPlaceholder]);

const additionalActions = useMemo(
() => (
<>
{currentLayout === "table" && tableLayoutOptions && (
<TableSettings
setTableState={setTableState}
tableLayoutOptions={tableLayoutOptions}
tableState={tableState}
/>
)}

{availableLayouts.length > 1 && (
<LayoutSwitcher
availableLayouts={availableLayouts}
currentLayout={currentLayout}
setCurrentLayout={setCurrentLayout}
/>
)}

{additionalActionButton}

{additionalActionMenuItems && (
<MenuButton
endIcon={<MoreIcon />}
ariaLabel={t("table.moreactions.arialabel")}
buttonVariant="secondary"
menuAlignment="right"
>
{additionalActionMenuItems}
</MenuButton>
)}
</>
),
[
currentLayout,
tableLayoutOptions,
tableState,
availableLayouts,
additionalActionButton,
additionalActionMenuItems,
t,
],
);
const additionalActions = useMemo(() => {
return (
(metaText ||
Comment on lines +285 to +287
Copy link
Contributor

@KevinGhadyani-Okta KevinGhadyani-Okta Sep 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make this an implicit return right?

But even then, there's a lot going on in here.

Is there a reason this isn't just another component?

React v19 would make this separate useMemo hook, but we don't need to do it here. It's only boolean checks.

This just seems like an unnecessary optimization by moving the boolean render of these action buttons up here rather than down in the return where we'd expect to see it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of making this another component, but I don't have time to tackle that right now. Would you be comfortable with us shipping as-is and then doing a fast-follow PR to break this out into a new component?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes because it's more of a code styling thing. It doesn't affect consumers.

(currentLayout === "table" && tableLayoutOptions) ||
availableLayouts.length > 1 ||
additionalActionButton ||
additionalActionMenuItems) && (
<AdditionalActionsInner odysseyDesignTokens={odysseyDesignTokens}>
{metaText && (
<MetaTextContainer odysseyDesignTokens={odysseyDesignTokens}>
<Typography color="textSecondary">{metaText}</Typography>
</MetaTextContainer>
)}

{currentLayout === "table" && tableLayoutOptions && (
<TableSettings
setTableState={setTableState}
tableLayoutOptions={tableLayoutOptions}
tableState={tableState}
/>
)}

{availableLayouts.length > 1 && (
<LayoutSwitcher
availableLayouts={availableLayouts}
currentLayout={currentLayout}
setCurrentLayout={setCurrentLayout}
/>
)}

{additionalActionButton}

{additionalActionMenuItems && (
<MenuButton
endIcon={<MoreIcon />}
ariaLabel={t("table.moreactions.arialabel")}
buttonVariant="secondary"
menuAlignment="right"
>
{additionalActionMenuItems}
</MenuButton>
)}
</AdditionalActionsInner>
)
);
}, [
odysseyDesignTokens,
metaText,
currentLayout,
tableLayoutOptions,
tableState,
availableLayouts,
additionalActionButton,
additionalActionMenuItems,
t,
]);

const { lastRow: lastRowOnPage } = usePagination({
currentRowsCount: data.length,
Expand Down Expand Up @@ -356,11 +385,14 @@ const DataView = ({
</BulkActionsContainer>
)}

{!shouldShowFilters && !bulkActionMenuItems && !hasRowSelection && (
<AdditionalActionsContainer>
{additionalActions}
</AdditionalActionsContainer>
)}
{!shouldShowFilters &&
!bulkActionMenuItems &&
!hasRowSelection &&
additionalActions && (
<AdditionalActionsContainer>
{additionalActions}
</AdditionalActionsContainer>
)}

{currentLayout === "table" && tableLayoutOptions && (
<TableLayoutContent
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export type UniversalProps = {
isRowReorderingDisabled?: boolean;
maxPages?: number;
maxResultsPerPage?: number;
metaText?: string;
noResultsPlaceholder?: ReactNode;
onChangeRowSelection?: (rowSelection: DataRowSelectionState) => void;
onReorderRows?: ({ rowId, newRowIndex }: DataOnReorderRowsType) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,9 @@ const storybookMeta: Meta<DataViewMetaProps> = {
currentPage: {
control: "number",
},
metaText: {
control: "text",
},
paginationType: {
control: "select",
options: paginationTypeValues,
Expand Down Expand Up @@ -413,6 +416,7 @@ const BaseStory: StoryObj<DataViewMetaProps> = {
hasSearch={args.hasSearch}
hasSearchSubmitButton={args.hasSearchSubmitButton}
isPaginationMoreDisabled={args.isPaginationMoreDisabled}
metaText={args.metaText}
searchDelayTime={args.searchDelayTime}
errorMessage={args.errorMessage}
initialLayout={args.initialLayout}
Expand Down Expand Up @@ -502,6 +506,7 @@ export const Everything: StoryObj<DataViewMetaProps> = {
hasRowSelection: true,
hasAdditionalActionButton: true,
hasAdditionalActionMenuItems: true,
metaText: "Last updated 12 hours ago",
},
};

Expand Down