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

#9250 Refactor ModActionItem remove unnecessary props #9254

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 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
88 changes: 48 additions & 40 deletions src/pageEditor/modListingPanel/ModActionMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,79 +28,87 @@ import styles from "./ActionMenu.module.scss";
import EllipsisMenu, {
type EllipsisMenuItem,
} from "@/components/ellipsisMenu/EllipsisMenu";
import { type AddNewModComponent } from "@/pageEditor/hooks/useAddNewModComponent";
import useAddNewModComponent from "@/pageEditor/hooks/useAddNewModComponent";
import { useAvailableFormStateAdapters } from "@/pageEditor/starterBricks/adapter";
import useDeactivateMod from "@/pageEditor/hooks/useDeactivateMod";
import useSaveMod from "@/pageEditor/hooks/useSaveMod";
import { type ModMetadata } from "@/types/modComponentTypes";
import useClearModChanges from "@/pageEditor/hooks/useClearModChanges";
import { isInnerDefinitionRegistryId } from "@/types/helpers";
import { actions } from "@/pageEditor/store/editor/editorSlice";
import { useDispatch, useSelector } from "react-redux";
import {
selectActiveModId,
selectModIsDirty,
} from "@/pageEditor/store/editor/editorSelectors";

type OptionalAction = (() => Promise<void>) | undefined;

type ActionMenuProps = {
isDirty: boolean;
isActive: boolean;
const ModActionMenu: React.FC<{
modMetadata: ModMetadata;
labelRoot: string;
onDeactivate: () => Promise<void>;
onMakeCopy: () => Promise<void>;
onAddStarterBrick: AddNewModComponent;
// Actions only defined if there are changes
onSave: OptionalAction;
onClearChanges: OptionalAction;
};
}> = ({ modMetadata, labelRoot }) => {
const { id: modId } = modMetadata;
const activeModId = useSelector(selectActiveModId);

const ModActionMenu: React.FC<ActionMenuProps> = ({
isActive,
labelRoot,
isDirty,
onAddStarterBrick,
onDeactivate,
onMakeCopy,
// Convert to null because EllipsisMenuItem expects null vs. undefined
onSave = null,
onClearChanges = null,
}) => {
const dispatch = useDispatch();
const modComponentFormStateAdapters = useAvailableFormStateAdapters();

const deactivateMod = useDeactivateMod();
const saveMod = useSaveMod();
const clearModChanges = useClearModChanges();
const addNewModComponent = useAddNewModComponent(modMetadata);

const isUnsavedMod = isInnerDefinitionRegistryId(modId);
const isDirty = useSelector(selectModIsDirty(modId));
const isActive = activeModId === modId;

const menuItems: EllipsisMenuItem[] = [
{
title: "Clear Changes",
icon: <FontAwesomeIcon icon={faHistory} fixedWidth />,
action: onClearChanges,
// Always show Clear Changes button, even if there are no changes so the UI is more consistent / the user doesn't
// wonder why the menu item is missing
disabled: !isDirty || !onClearChanges,
async action() {
await clearModChanges(modId);
},
// Always show Clear Changes button, even if there are no changes or the mod is an unsaved mod so the UI is more
// consistent / the user doesn't wonder why the menu item is missing
disabled: !isDirty || isUnsavedMod,
},
{
title: "Add Starter Brick",
icon: <FontAwesomeIcon icon={faPlus} fixedWidth />,
submenu: modComponentFormStateAdapters.map((adapter) => ({
title: adapter.label,
action() {
onAddStarterBrick(adapter);
addNewModComponent(adapter);
},
icon: <FontAwesomeIcon icon={adapter.icon} fixedWidth />,
})),
hide: !onAddStarterBrick,
},
{
title: "Make a copy",
icon: <FontAwesomeIcon icon={faClone} fixedWidth />,
action: onMakeCopy,
async action() {
dispatch(actions.showCreateModModal({ keepLocalCopy: true }));
},
},
{
title: "Deactivate",
icon: <FontAwesomeIcon icon={faTimes} fixedWidth />,
action: onDeactivate,
hide: !onDeactivate,
async action() {
await deactivateMod({ modId });
},
},
];

return (
<div className={styles.root}>
{onSave != null && (
<SaveButton
ariaLabel={labelRoot ? `${labelRoot} - Save` : undefined}
onClick={onSave}
disabled={!isDirty}
/>
)}
{/* TODO: did we really want to always show SaveButton? That is the current behavior as of 2.1.5-beta.1 */}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we? I noticed that the disabled save button might look a little awkward, but maybe that's just me

Screenshot 2024-10-08 at 6 28 11 PM

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I would actually sooner expect the ellipsis menu to always show, rather than the save button

<SaveButton
ariaLabel={labelRoot ? `${labelRoot} - Save` : undefined}
onClick={async () => {
await saveMod(modId);
}}
disabled={!isDirty}
/>
{isActive && (
<EllipsisMenu
portal
Expand Down
28 changes: 2 additions & 26 deletions src/pageEditor/modListingPanel/ModComponents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,17 +33,12 @@ import {
selectNotDeletedModComponentFormStates,
selectNotDeletedActivatedModComponents,
} from "@/pageEditor/store/editor/editorSelectors";
import { useDispatch, useSelector } from "react-redux";
import useSaveMod from "@/pageEditor/hooks/useSaveMod";
import useClearModChanges from "@/pageEditor/hooks/useClearModChanges";
import useDeactivateMod from "@/pageEditor/hooks/useDeactivateMod";
import { useSelector } from "react-redux";
import ModComponentListItem from "./ModComponentListItem";
import { actions } from "@/pageEditor/store/editor/editorSlice";
import { useDebounce } from "use-debounce";
import filterSidebarItems from "@/pageEditor/modListingPanel/filterSidebarItems";

const ModComponents: React.FunctionComponent = () => {
const dispatch = useDispatch();
const activeModComponentId = useSelector(selectActiveModComponentId);
const activeModId = useSelector(selectActiveModId);
const expandedModId = useSelector(selectExpandedModId);
Expand Down Expand Up @@ -87,31 +82,12 @@ const ModComponents: React.FunctionComponent = () => {
],
);

const saveMod = useSaveMod();
const clearModChanges = useClearModChanges();
const deactivateMod = useDeactivateMod();

const listItems = filteredSidebarItems.map((sidebarItem) => {
if (isModSidebarItem(sidebarItem)) {
const { modMetadata, modComponents } = sidebarItem;

return (
<ModListItem
key={modMetadata.id}
modMetadata={modMetadata}
onSave={async () => {
await saveMod(modMetadata.id);
}}
onClearChanges={async () => {
await clearModChanges(modMetadata.id);
}}
onDeactivate={async () => {
await deactivateMod({ modId: modMetadata.id });
}}
onMakeCopy={async () => {
dispatch(actions.showCreateModModal({ keepLocalCopy: true }));
}}
>
<ModListItem key={modMetadata.id} modMetadata={modMetadata}>
{modComponents.map((modComponentSidebarItem) => (
<ModComponentListItem
key={getModComponentItemId(modComponentSidebarItem)}
Expand Down
24 changes: 3 additions & 21 deletions src/pageEditor/modListingPanel/ModListItem.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,7 @@ describe("ModListItem", () => {
render(
<Accordion defaultActiveKey={modMetadata.id}>
<ListGroup>
<ModListItem
modMetadata={modMetadata}
onSave={jest.fn()}
onClearChanges={jest.fn()}
onDeactivate={jest.fn()}
onMakeCopy={jest.fn()}
>
<ModListItem modMetadata={modMetadata}>
<div>test children</div>
</ModListItem>
</ListGroup>
Expand All @@ -69,13 +63,7 @@ describe("ModListItem", () => {
render(
<Accordion>
<ListGroup>
<ModListItem
modMetadata={modMetadata}
onSave={jest.fn()}
onClearChanges={jest.fn()}
onDeactivate={jest.fn()}
onMakeCopy={jest.fn()}
>
<ModListItem modMetadata={modMetadata}>
<div>test children</div>
</ModListItem>
</ListGroup>
Expand Down Expand Up @@ -109,13 +97,7 @@ describe("ModListItem", () => {
render(
<Accordion defaultActiveKey={modMetadata.id}>
<ListGroup>
<ModListItem
modMetadata={modMetadata}
onSave={jest.fn()}
onClearChanges={jest.fn()}
onDeactivate={jest.fn()}
onMakeCopy={jest.fn()}
>
<ModListItem modMetadata={modMetadata}>
<div>test children</div>
</ModListItem>
</ListGroup>
Expand Down
31 changes: 2 additions & 29 deletions src/pageEditor/modListingPanel/ModListItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,38 +33,23 @@ import {
selectActiveModId,
selectDirtyMetadataForModId,
selectExpandedModId,
selectModIsDirty,
} from "@/pageEditor/store/editor/editorSelectors";
import * as semver from "semver";
import { useGetModDefinitionQuery } from "@/data/service/api";
import useAddNewModComponent from "@/pageEditor/hooks/useAddNewModComponent";
import { type ModMetadata } from "@/types/modComponentTypes";
import { isInnerDefinitionRegistryId } from "@/types/helpers";
import ModActionMenu from "@/pageEditor/modListingPanel/ModActionMenu";

export type ModListItemProps = PropsWithChildren<{
modMetadata: ModMetadata;
onSave: () => Promise<void>;
onClearChanges: () => Promise<void>;
onDeactivate: () => Promise<void>;
onMakeCopy: () => Promise<void>;
}>;

const ModListItem: React.FC<ModListItemProps> = ({
modMetadata,
children,
onSave,
onClearChanges,
onDeactivate,
onMakeCopy,
}) => {
const ModListItem: React.FC<ModListItemProps> = ({ modMetadata, children }) => {
const dispatch = useDispatch();
const activeModId = useSelector(selectActiveModId);
const expandedModId = useSelector(selectExpandedModId);
const activeModComponentFormState = useSelector(
selectActiveModComponentFormState,
);
const addNewModComponent = useAddNewModComponent(modMetadata);

const { id: modId, name: savedName, version: activatedVersion } = modMetadata;
const isActive = activeModId === modId;
Expand All @@ -80,9 +65,6 @@ const ModListItem: React.FC<ModListItemProps> = ({

const dirtyName = useSelector(selectDirtyMetadataForModId(modId))?.name;
const name = dirtyName ?? savedName ?? "Loading...";
const isDirty = useSelector(selectModIsDirty(modId));

const isUnsavedMod = isInnerDefinitionRegistryId(modMetadata.id);

const hasUpdate =
latestModVersion != null &&
Expand Down Expand Up @@ -115,16 +97,7 @@ const ModListItem: React.FC<ModListItemProps> = ({
/>
</span>
)}
<ModActionMenu
isActive={isActive}
labelRoot={name}
onSave={onSave}
onClearChanges={isUnsavedMod ? undefined : onClearChanges}
onDeactivate={onDeactivate}
onAddStarterBrick={addNewModComponent}
onMakeCopy={onMakeCopy}
isDirty={isDirty}
/>
<ModActionMenu modMetadata={modMetadata} labelRoot={name} />
</Accordion.Toggle>
<Accordion.Collapse eventKey={modId}>
<>{children}</>
Expand Down
Loading