Skip to content

Commit

Permalink
rework project and user settings component, other small code review f…
Browse files Browse the repository at this point in the history
…ixes
  • Loading branch information
Jolie Rabideau authored and Jolie Rabideau committed Aug 21, 2024
1 parent 73f71e8 commit 6d64dc3
Show file tree
Hide file tree
Showing 10 changed files with 305 additions and 229 deletions.
86 changes: 43 additions & 43 deletions lib/platform-bible-react/dist/index.cjs

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion lib/platform-bible-react/dist/index.cjs.map

Large diffs are not rendered by default.

90 changes: 47 additions & 43 deletions lib/platform-bible-react/dist/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/platform-bible-react/dist/index.js.map

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ export default function MoreInfo({
<div className="pr-ml-auto pr-flex pr-flex-col pr-space-y-2">
<a
href={moreInfoUrl}
target="_blank"
rel="noreferrer"
className="pr-flex pr-items-center pr-text-xs pr-font-semibold pr-text-gray-500 pr-underline"
>
Website
<LucideLink className="pr-ml-1 pr-inline pr-h-4 pr-w-4" />
</a>
<a
href="https://placeholder.com"
href="https://example.com"
target="_blank"
rel="noreferrer"
className="pr-flex pr-items-center pr-text-xs pr-font-semibold pr-text-gray-500 pr-underline"
>
Support
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@
"start:extension-host": "cross-env NODE_ENV=development nodemon --transpile-only ./src/extension-host/extension-host.ts",
"start:extensions": "cd extensions && npm run watch",
"start:preload": "cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.preload.dev.ts",
"start:renderer": "DEV_NOISY=true cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack serve --config ./.erb/configs/webpack.config.renderer.dev.ts",
"start:renderer": "cross-env NODE_ENV=development TS_NODE_TRANSPILE_ONLY=true webpack serve --config ./.erb/configs/webpack.config.renderer.dev.ts",
"start:data": "dotnet watch --project c-sharp/ParanextDataProvider.csproj",
"stop": "tsx stop-processes.mjs",
"test:core": "jest --silent",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,28 @@ import { Localized, ProjectSettingProperties, SettingProperties } from 'platform
import { SettingsList, SettingsListHeader } from 'platform-bible-react';
import { useMemo } from 'react';
import { ProjectSettingNames, SettingNames } from 'papi-shared-types';
import Setting, { SettingValues } from './setting.component';
import ProjectSetting, { ProjectSettingValues } from './project-setting.component';
import UserSetting from './user-setting.component';
import ProjectSetting from './project-setting.component';
import { ProjectSettingValues, UserSettingValues } from './setting.component';

/** Properties for a settings list component that displays either project or user settings */
type ProjectOrUserSettingsListProps = {
/** Properties for either a project setting group or user setting group */
settingProperties: Localized<ProjectSettingProperties> | Localized<SettingProperties>;
/** Optional projectId, supplied if the list is for project settings */
projectId?: string;
/** Current search query to filter the list by */
searchQuery: string;
/** Settings group label */
groupLabel: string;
/** Optional settings group description */
groupDescription?: string;
};

/**
* Filters and displays a list of settings based on a search query and whether it's for project or
* user settings, rendering either `ProjectSetting` or `UserSetting` components accordingly.
*/
export default function ProjectOrUserSettingsList({
settingProperties,
projectId,
Expand Down Expand Up @@ -56,7 +67,7 @@ export default function ProjectOrUserSettingsList({
defaultSetting={property.default as ProjectSettingValues}
/>
) : (
<Setting
<UserSetting
key={key}
// Key is a string technically, but it has to be a settingKey to access the setting
// eslint-disable-next-line no-type-assertion/no-type-assertion
Expand All @@ -65,7 +76,7 @@ export default function ProjectOrUserSettingsList({
description={property.description}
// Default is unknown technically, but we know it has to be a setting value
// eslint-disable-next-line no-type-assertion/no-type-assertion
defaultSetting={property.default as SettingValues}
defaultSetting={property.default as UserSettingValues}
/>
),
)}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,21 +1,12 @@
import { ChangeEvent, useCallback, useState } from 'react';
import { useProjectSetting } from '@renderer/hooks/papi-hooks';
import { ProjectSettingNames, ProjectSettingTypes } from 'papi-shared-types';
import projectSettingsService from '@shared/services/project-settings.service';
import { Checkbox, Input, SettingsListItem } from 'platform-bible-react';
import { debounce } from 'platform-bible-utils';

/** Values of the ProjectSettingTypes */
export type ProjectSettingValues = ProjectSettingTypes[keyof ProjectSettingTypes];

type ProjectSettingProps = {
settingKey: ProjectSettingNames;
label: string;
description?: string;
projectId: string;
defaultSetting: ProjectSettingValues;
};
import Setting, { ProjectSettingProps, ProjectSettingValues } from './setting.component';

/**
* Provides a project-specific setting component by using the `Setting` component with
* project-specific validation
*/
export default function ProjectSetting({
settingKey,
label,
Expand All @@ -29,80 +20,23 @@ export default function ProjectSetting({
defaultSetting,
);

const [errorMessage, setErrorMessage] = useState<string | undefined>(undefined);

const handleChangeSetting = async (event: ChangeEvent<HTMLInputElement>) => {
let newValue: ProjectSettingValues =
event.target.type === 'checkbox' ? event.target.checked : event.target.value;

if (typeof setting === 'number') {
// If setting is a number the response will be a string, but newValue is
// ProjectSettingValues so TS doesn't know that
// eslint-disable-next-line no-type-assertion/no-type-assertion
const numericValue = parseInt(newValue as string, 10);
if (Number.isNaN(numericValue)) {
setErrorMessage('Invalid number');
return;
}
newValue = numericValue;
}

try {
if (await projectSettingsService.isValid(settingKey, newValue, setting)) {
setErrorMessage(undefined);
if (setSetting) setSetting(newValue);
} else {
setErrorMessage('Invalid value');
}
} catch (error) {
// Error is type unknown and if I type it as string it says it must be any or unknown
// eslint-disable-next-line no-type-assertion/no-type-assertion
setErrorMessage(error as string);
}
const validateProjectSetting = async (
currentSettingKey: ProjectSettingNames,
newValue: ProjectSettingValues,
currentValue: ProjectSettingValues,
) => {
return projectSettingsService.isValid(currentSettingKey, newValue, currentValue);
};

const debouncedHandleChange = debounce(handleChangeSetting, 500);

const generateComponent = useCallback(() => {
let component = <p>No setting component</p>;

if (typeof setting === 'string' || typeof setting === 'number')
component = (
<Input key={settingKey} onChange={debouncedHandleChange} defaultValue={setting} />
);
else if (typeof setting === 'boolean')
component = (
<Checkbox
key={settingKey}
onChange={() => debouncedHandleChange}
isDefaultChecked={setting}
/>
);
else if (typeof setting === 'object')
component = (
<Input
key={settingKey}
onChange={debouncedHandleChange}
defaultValue={JSON.stringify(setting, undefined, 2)}
/>
);

return (
<div>
{component}
{errorMessage && <div style={{ color: 'red' }}>{errorMessage}</div>}
</div>
);
}, [setting, settingKey, debouncedHandleChange, errorMessage]);

return (
<SettingsListItem
primary={label}
secondary={description}
<Setting
settingKey={settingKey}
setting={setting}
setSetting={setSetting}
isLoading={isLoading}
loadingMessage="Loading setting"
>
{generateComponent()}
</SettingsListItem>
validateProjectSetting={validateProjectSetting}
label={label}
description={description}
/>
);
}
Loading

0 comments on commit 6d64dc3

Please sign in to comment.