Skip to content

Commit

Permalink
Merge pull request #1203 from qdraw/feature/202308_code_smells_01
Browse files Browse the repository at this point in the history
code smells
  • Loading branch information
qdraw authored Aug 1, 2023
2 parents d0dfefd + b40d0d4 commit 4fd559e
Show file tree
Hide file tree
Showing 14 changed files with 119 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ public static FileIndexItem Set(FileIndexItem? sourceIndexItem, string fieldName
/// </summary>
/// <param name="fieldName">name e.g. Tags</param>
/// <returns>bool, true=exist</returns>
[SuppressMessage("Usage", "Collection-specific Exists " +
[SuppressMessage("Usage", "S6605: Collection-specific Exists " +
"method should be used instead of the Any extension.")]
public static bool CheckIfPropertyExist(string fieldName)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,14 @@ export class LimitLength {

const elementLength = element.currentTarget.textContent.trim().length;

const anyKeyRegex = new RegExp(/^.?$/);

if (
elementLength < this.maxlength ||
window.getSelection()?.type === "Range" ||
(element.key === "x" && element.ctrlKey) ||
(element.key === "x" && element.metaKey) ||
!element.key.match(/^.?$/)
!anyKeyRegex.exec(element.key)
)
return;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,9 @@ const ArchiveSidebarSelectionList: React.FunctionComponent<IDetailViewSidebarSel
{select
? select.map(
(
item,
index // item is filename
item // item is filename
) => (
<li key={index}>
<li key={item}>
<span
onClick={() => toggleSelection(item)}
className="close"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,20 @@ interface ItemListProps {
isLoading?: boolean;
callback?(path: string): void;
}

function GetBoxClass(item: IFileIndexItem): string {
if (item.isDirectory) {
return "box isDirectory-true";
} else if (
item.status === IExifStatus.Ok ||
item.status === IExifStatus.Default
) {
return "box isDirectory-false";
} else {
return "box isDirectory-false error";
}
}

/**
* A list with links to the items
*/
Expand Down Expand Up @@ -47,14 +61,7 @@ const ItemTextListView: React.FunctionComponent<ItemListProps> = (props) => {
<ul>
{props.fileIndexItems.map((item, index) => (
<li
className={
item.isDirectory
? "box isDirectory-true"
: item.status === IExifStatus.Ok ||
item.status === IExifStatus.Default
? "box isDirectory-false"
: "box isDirectory-false error"
}
className={GetBoxClass(item)}
key={item.filePath + item.lastEdited}
>
{item.isDirectory ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@ import Notification, {
} from "../../atoms/notification/notification";
import Preloader from "../../atoms/preloader/preloader";

function GetVideoClass(isPaused: boolean, isStarted: boolean): string {
if (isPaused) {
if (isStarted) {
return "video play";
} else {
return "video first";
}
} else {
return "video pause";
}
}

const DetailViewMp4: React.FunctionComponent = memo(() => {
// content
const settings = useGlobalSettings();
Expand Down Expand Up @@ -219,13 +231,7 @@ const DetailViewMp4: React.FunctionComponent = memo(() => {
{!isError ? (
<figure
data-test="video"
className={
isPaused
? isStarted
? "video play"
: "video first"
: "video pause"
}
className={GetVideoClass(isPaused, isStarted)}
onClick={() => {
playPause();
timeUpdate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const DetailViewInfoDateTime: React.FunctionComponent<IDetailViewInfoDateTimePro

function handleExit(result: IFileIndexItem[] | null) {
setModalDatetimeOpen(false);
if (!result || !result[0]) return;
if (!result?.[0]) return;
// only update the content that can be changed
setFileIndexItem({
...fileIndexItem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { newIFileIndexItemArray } from "../../../interfaces/IFileIndexItem";
import localization from "../../../localization/localization.json";
import { FileListCache } from "../../../shared/filelist-cache";
import { Language } from "../../../shared/language";
import { GetArchiveSearchMenuHeaderClass } from "../../../shared/menu/get-archive-search-menu-header-class";
import { Select } from "../../../shared/select";
import { Sidebar } from "../../../shared/sidebar";
import { URLPath } from "../../../shared/url-path";
Expand Down Expand Up @@ -214,15 +215,7 @@ const MenuArchive: React.FunctionComponent<IMenuArchiveProps> = memo(() => {
/>

{/* Menu */}
<header
className={
sidebar
? "header header--main header--select header--edit"
: select
? "header header--main header--select"
: "header header--main "
}
>
<header className={GetArchiveSearchMenuHeaderClass(sidebar, select)}>
<div className="wrapper">
<HamburgerMenuToggle
select={select}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,25 @@ export interface MenuDetailViewProps {
dispatch: React.Dispatch<DetailViewAction>;
}

function GetHeaderClass(
isDetails: boolean | undefined,
isMarkedAsDeleted: boolean
): string {
if (isDetails) {
if (isMarkedAsDeleted) {
return "header header--main header--edit header--deleted";
} else {
return "header header--main header--edit";
}
} else {
if (isMarkedAsDeleted) {
return "header header--main header--deleted";
} else {
return "header header--main";
}
}
}

const MenuDetailView: React.FunctionComponent<MenuDetailViewProps> = ({
state,
dispatch
Expand Down Expand Up @@ -244,10 +263,9 @@ const MenuDetailView: React.FunctionComponent<MenuDetailViewProps> = ({
return null;
}
const media = new CastToInterface().MediaDetailView(resultGet.data).data;
const orientation =
media.fileIndexItem && media.fileIndexItem.orientation
? media.fileIndexItem.orientation
: Orientation.Horizontal;
const orientation = media?.fileIndexItem?.orientation
? media.fileIndexItem.orientation
: Orientation.Horizontal;

// the hash changes if you rotate an image
if (media.fileIndexItem.fileHash === state.fileIndexItem.fileHash)
Expand Down Expand Up @@ -366,17 +384,7 @@ const MenuDetailView: React.FunctionComponent<MenuDetailViewProps> = ({
setModalPublishOpen={setModalPublishOpen}
/>

<header
className={
isDetails
? isMarkedAsDeleted
? "header header--main header--edit header--deleted"
: "header header--main header--edit"
: isMarkedAsDeleted
? "header header--main header--deleted"
: "header header--main"
}
>
<header className={GetHeaderClass(isDetails, isMarkedAsDeleted)}>
<div className="wrapper">
{/* in directory state aka no search */}
{!isSearchQuery ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import useLocation from "../../../hooks/use-location";
import { IArchiveProps } from "../../../interfaces/IArchiveProps";
import localization from "../../../localization/localization.json";
import { Language } from "../../../shared/language";
import { GetArchiveSearchMenuHeaderClass } from "../../../shared/menu/get-archive-search-menu-header-class";
import { Select } from "../../../shared/select";
import { Sidebar } from "../../../shared/sidebar";
import { URLPath } from "../../../shared/url-path";
Expand Down Expand Up @@ -113,15 +114,7 @@ export const MenuSearch: React.FunctionComponent<IMenuSearchProps> = ({
setModalPublishOpen={setModalPublishOpen}
/>

<header
className={
sidebar
? "header header--main header--select header--edit"
: select
? "header header--main header--select"
: "header header--main "
}
>
<header className={GetArchiveSearchMenuHeaderClass(sidebar, select)}>
<div className="wrapper">
<HamburgerMenuToggle
select={select}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,17 @@ const ModalGeo: React.FunctionComponent<IModalMoveFileProps> = ({
}, []);

function subHeader(): string {
return isFormEnabled
? !latitude && !longitude
? MessageAddLocation
: MessageUpdateLocation
: MessageViewLocation;
let message: string;
if (isFormEnabled) {
if (!latitude && !longitude) {
message = MessageAddLocation;
} else {
message = MessageUpdateLocation;
}
} else {
message = MessageViewLocation;
}
return message;
}

function updateButton(): React.JSX.Element {
Expand Down
4 changes: 2 additions & 2 deletions starsky/starsky/clientapp/src/contexts/archive-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ function updateCache(stateLocal: IArchiveProps): IArchiveProps {
function ArchiveContextProvider({ children }: ReactNodeProps) {
// [A]
let [state, dispatch] = React.useReducer(archiveReducer, initialState);
let value1 = { state, dispatch };
let value1 = React.useMemo(() => ({ state, dispatch }), [state, dispatch]);

// [B]
return (
Expand All @@ -461,7 +461,7 @@ function ArchiveContextProvider({ children }: ReactNodeProps) {
let ArchiveContextConsumer = ArchiveContext.Consumer;

// [C]
export { ArchiveContext, ArchiveContextProvider, ArchiveContextConsumer };
export { ArchiveContext, ArchiveContextConsumer, ArchiveContextProvider };

// exporter
export const useArchiveContext = () => React.useContext(ArchiveContext);
Expand Down
9 changes: 5 additions & 4 deletions starsky/starsky/clientapp/src/contexts/detailview-context.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React from "react";
import React, { useMemo } from "react";
import {
IDetailView,
IRelativeObjects,
Expand Down Expand Up @@ -157,7 +157,8 @@ function updateCache(stateLocal: IDetailView): IDetailView {
function DetailViewContextProvider({ children }: ReactNodeProps) {
// [A]
let [state, dispatch] = React.useReducer(detailviewReducer, initialState);
let value1 = { state, dispatch };
// Use useMemo to memoize the value object
let value1 = useMemo(() => ({ state, dispatch }), [state, dispatch]);

// [B]
return (
Expand All @@ -171,8 +172,8 @@ let DetailViewContextConsumer = DetailViewContext.Consumer;

export {
DetailViewContext,
DetailViewContextProvider,
DetailViewContextConsumer
DetailViewContextConsumer,
DetailViewContextProvider
};

export const useDetailViewContext = () => React.useContext(DetailViewContext);
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { GetArchiveSearchMenuHeaderClass } from "./get-archive-search-menu-header-class";

describe("GetArchiveSearchMenuHeaderClass", () => {
it("returns the correct class when sidebar is true", () => {
const sidebar = true;
const select = undefined;
const expectedClass = "header header--main header--select header--edit";
const result = GetArchiveSearchMenuHeaderClass(sidebar, select);
expect(result).toEqual(expectedClass);
});

it("returns the correct class when select is not empty", () => {
const sidebar = false;
const select = ["option1", "option2"];
const expectedClass = "header header--main header--select";
const result = GetArchiveSearchMenuHeaderClass(sidebar, select);
expect(result).toEqual(expectedClass);
});

it("returns the correct class when sidebar and select are falsey", () => {
const sidebar = false;
const select = undefined;
const expectedClass = "header header--main";
const result = GetArchiveSearchMenuHeaderClass(sidebar, select);
expect(result).toEqual(expectedClass);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export function GetArchiveSearchMenuHeaderClass(
sidebar: boolean | undefined,
select: string[] | undefined
): string {
if (sidebar) {
return "header header--main header--select header--edit";
} else if (select) {
return "header header--main header--select";
} else {
return "header header--main";
}
}

1 comment on commit 4fd559e

@github-actions
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.