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.

Sign up for GitHub

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

migrate BudgetList to typescript #3026

Merged
merged 3 commits into from
Jul 13, 2024
Merged

migrate BudgetList to typescript #3026

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
647e2bd
migrate BudgetList to typescript
tcrasset Jul 10, 2024
587bdde
run linter
tcrasset Jul 10, 2024
54cc2fa
add release notes
tcrasset Jul 10, 2024
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
134 changes: 93 additions & 41 deletions ...ent/src/components/manager/BudgetList.jsx → ...ent/src/components/manager/BudgetList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useState, useRef } from 'react';
import type React from 'react';
import { useState, useRef, type CSSProperties } from 'react';
import { useDispatch, useSelector } from 'react-redux';

import {
Expand All @@ -12,6 +13,12 @@ import {
pushModal,
} from 'loot-core/client/actions';
import { isNonProductionEnvironment } from 'loot-core/src/shared/environment';
import {
type File,
type LocalFile,
type SyncableLocalFile,
type SyncedLocalFile,
} from 'loot-core/types/file';

import { useInitialMount } from '../../hooks/useInitialMount';
import { useLocalPref } from '../../hooks/useLocalPref';
Expand All @@ -32,7 +39,7 @@ import { Popover } from '../common/Popover';
import { Text } from '../common/Text';
import { View } from '../common/View';

function getFileDescription(file) {
function getFileDescription(file: File) {
if (file.state === 'unknown') {
return (
'This is a cloud-based file but its state is unknown because you ' +
Expand All @@ -50,8 +57,14 @@ function getFileDescription(file) {
return null;
}

function FileMenu({ onDelete, onClose }) {
function onMenuSelect(type) {
function FileMenu({
onDelete,
onClose,
}: {
onDelete: () => void;
onClose: () => void;
}) {
function onMenuSelect(type: string) {
onClose();

switch (type) {
Expand Down Expand Up @@ -83,7 +96,7 @@ function FileMenu({ onDelete, onClose }) {
);
}

function FileMenuButton({ state, onDelete }) {
function FileMenuButton({ onDelete }: { onDelete: () => void }) {
const triggerRef = useRef(null);
const [menuOpen, setMenuOpen] = useState(false);

Expand All @@ -106,17 +119,13 @@ function FileMenuButton({ state, onDelete }) {
isOpen={menuOpen}
onOpenChange={() => setMenuOpen(false)}
>
<FileMenu
state={state}
onDelete={onDelete}
onClose={() => setMenuOpen(false)}
/>
<FileMenu onDelete={onDelete} onClose={() => setMenuOpen(false)} />
</Popover>
</View>
);
}

function FileState({ file }) {
function FileState({ file }: { file: File }) {
let Icon;
let status;
let color;
Expand Down Expand Up @@ -164,10 +173,20 @@ function FileState({ file }) {
);
}

function File({ file, quickSwitchMode, onSelect, onDelete }) {
function FileItem({
file,
quickSwitchMode,
onSelect,
onDelete,
}: {
file: File;
quickSwitchMode: boolean;
onSelect: (file: File) => void;
onDelete: (file: File) => void;
}) {
const selecting = useRef(false);

async function _onSelect(file) {
async function _onSelect(file: File) {
// Never allow selecting the file while uploading/downloading, and
// make sure to never allow duplicate clicks
if (!selecting.current) {
Expand All @@ -180,7 +199,7 @@ function File({ file, quickSwitchMode, onSelect, onDelete }) {
return (
<View
onClick={() => _onSelect(file)}
title={getFileDescription(file)}
title={getFileDescription(file) || ''}
style={{
flexDirection: 'row',
justifyContent: 'space-between',
Expand All @@ -193,7 +212,7 @@ function File({ file, quickSwitchMode, onSelect, onDelete }) {
flexShrink: 0,
cursor: 'pointer',
':hover': {
backgroundColor: theme.hover,
backgroundColor: theme.menuItemBackgroundHover,
},
}}
>
Expand All @@ -219,15 +238,27 @@ function File({ file, quickSwitchMode, onSelect, onDelete }) {
/>
)}

{!quickSwitchMode && (
<FileMenuButton state={file.state} onDelete={() => onDelete(file)} />
)}
{!quickSwitchMode && <FileMenuButton onDelete={() => onDelete(file)} />}
</View>
</View>
);
}

function BudgetFiles({ files, quickSwitchMode, onSelect, onDelete }) {
function BudgetFiles({
files,
quickSwitchMode,
onSelect,
onDelete,
}: {
files: File[];
quickSwitchMode: boolean;
onSelect: (file: File) => void;
onDelete: (file: File) => void;
}) {
function isLocalFile(file: File): file is LocalFile {
return file.state === 'local';
}

return (
<View
style={{
Expand All @@ -252,8 +283,8 @@ function BudgetFiles({ files, quickSwitchMode, onSelect, onDelete }) {
</Text>
) : (
files.map(file => (
<File
key={file.id || file.cloudFileId}
<FileItem
key={isLocalFile(file) ? file.id : file.cloudFileId}
file={file}
quickSwitchMode={quickSwitchMode}
onSelect={onSelect}
Expand All @@ -265,7 +296,13 @@ function BudgetFiles({ files, quickSwitchMode, onSelect, onDelete }) {
);
}

function RefreshButton({ style, onRefresh }) {
function RefreshButton({
style,
onRefresh,
}: {
style?: CSSProperties;
onRefresh: () => void;
}) {
const [loading, setLoading] = useState(false);

async function _onRefresh() {
Expand All @@ -288,7 +325,13 @@ function RefreshButton({ style, onRefresh }) {
);
}

function BudgetListHeader({ quickSwitchMode, onRefresh }) {
function BudgetListHeader({
quickSwitchMode,
onRefresh,
}: {
quickSwitchMode: boolean;
onRefresh: () => void;
}) {
return (
<View
style={{
Expand All @@ -314,7 +357,14 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) {
const allFiles = useSelector(state => state.budgets.allFiles || []);
const [id] = useLocalPref('id');

const files = id ? allFiles.filter(f => f.id !== id) : allFiles;
// Remote files do not have the 'id' field
function isNonRemoteFile(
file: File,
): file is LocalFile | SyncableLocalFile | SyncedLocalFile {
return file.state !== 'remote';
}
const nonRemoteFiles = allFiles.filter(isNonRemoteFile);
const files = id ? nonRemoteFiles.filter(f => f.id !== id) : allFiles;

Comment on lines +360 to +367
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Didn't know if I should do this or check file.cloudFileId if it's a RemoteFile. I've just kept the things in their current form. I haven't thought much about the logic behind it all

const [creating, setCreating] = useState(false);
const { isNarrowWidth } = useResponsive();
Expand All @@ -324,7 +374,7 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) {
}
: {};

const onCreate = ({ testMode } = {}) => {
const onCreate = ({ testMode = false } = {}) => {
if (!creating) {
setCreating(true);
dispatch(createBudget({ testMode }));
Expand All @@ -341,6 +391,22 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) {
refresh();
}

const onSelect = (file: File): void => {
const isRemoteFile = file.state === 'remote';

if (!id) {
if (isRemoteFile) {
dispatch(downloadBudget(file.cloudFileId));
} else {
dispatch(loadBudget(file.id));
}
} else if (!isRemoteFile && file.id !== id) {
dispatch(closeAndLoadBudget(file.id));
} else if (isRemoteFile) {
dispatch(closeAndDownloadBudget(file.cloudFileId));
}
};

return (
<View
style={{
Expand All @@ -365,21 +431,7 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) {
<BudgetFiles
files={files}
quickSwitchMode={quickSwitchMode}
onSelect={file => {
if (!id) {
if (file.state === 'remote') {
dispatch(downloadBudget(file.cloudFileId));
} else {
dispatch(loadBudget(file.id));
}
} else if (file.id !== id) {
if (file.state === 'remote') {
dispatch(closeAndDownloadBudget(file.cloudFileId));
} else {
dispatch(closeAndLoadBudget(file.id));
}
}
}}
onSelect={onSelect}
onDelete={file => dispatch(pushModal('delete-budget', { file }))}
/>
{!quickSwitchMode && (
Expand Down Expand Up @@ -408,7 +460,7 @@ export function BudgetList({ showHeader = true, quickSwitchMode = false }) {

<Button
type="primary"
onClick={onCreate}
onClick={() => onCreate()}
style={{
...narrowButtonStyle,
marginLeft: 10,
Expand Down
4 changes: 3 additions & 1 deletion packages/loot-core/src/client/reducers/budgets.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ function reconcileFiles(
groupId,
deleted: false,
state: 'unknown',
hasKey: true,
};
}

Expand Down Expand Up @@ -85,10 +86,11 @@ function reconcileFiles(
groupId,
deleted: false,
state: 'broken',
hasKey: true,
};
}
} else {
return { ...localFile, deleted: false, state: 'local' };
return { ...localFile, deleted: false, state: 'local', hasKey: true };
}
});

Expand Down
2 changes: 2 additions & 0 deletions packages/loot-core/src/types/file.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@ export type FileState =

export type LocalFile = Omit<Budget, 'cloudFileId' | 'groupId'> & {
state: 'local';
hasKey: boolean;
};

export type SyncableLocalFile = Budget & {
cloudFileId: string;
groupId: string;
state: 'broken' | 'unknown';
hasKey: boolean;
};
Comment on lines 11 to 21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

File.hasKey was used a few times in BudgetList.jsx. I don't know how the logic for hasKey is determined for local files. Can they be encrypted? If yes, why didn't they have the hasKey property? I chose to add it, and default it to True. That's because, in my view, if the file is imported locally, and is encrypted, if the user managed to import it, it means he has the key.


export type SyncedLocalFile = Budget & {
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3026.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Maintenance
authors: [tcrasset]
---

Migrate BudgetList to Typescript
Loading