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

Jump to bottom

Application settings management phase 2 #1408

Merged
merged 5 commits into from
Jul 2, 2024
Merged

Application settings management phase 2 #1408

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7962b35
Application Settings Panel refactor
rohinp14 Jul 1, 2024
f75096a
Update panel to consume props from provider
rohinp14 Jul 2, 2024
d97b9be
Merge branch 'finos:main' into application-settings-management
rohinp14 Jul 2, 2024
4313b9f
Move showcase example to Shell
rohinp14 Jul 2, 2024
8124232
Showcase examples + settings form value fix
rohinp14 Jul 2, 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
54 changes: 47 additions & 7 deletions vuu-ui/packages/vuu-shell/src/application-provider/ApplicationContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,68 @@
import type { ThemeMode, VuuUser } from "@finos/vuu-utils";

Check failure on line 1 in vuu-ui/packages/vuu-shell/src/application-provider/ApplicationContext.tsx

View workflow job for this annotation

GitHub Actions / lint-and-typecheck 

'ThemeMode' is defined but never used. Allowed unused vars must match /^_$/u
import { createContext } from "react";
import { SettingsSchema } from "../application-settings/SettingsForm";

export interface CoreSettings {
themeMode: ThemeMode;
}
// export interface CoreSettings {
// themeMode: ThemeMode;
// }

const Guest: VuuUser = {
username: "unknown",
token: "",
};

export interface ApplicationContextProps {
changeSetting: (propertyName: string, value: unknown) => void;
settings: CoreSettings;
onApplicationSettingChanged: (propertyName: string, value: unknown) => void;
applicationSettings: Record<string, string | number | boolean>;
applicationSettingsSchema: SettingsSchema;
user: VuuUser;
}

export const ApplicationContext = createContext<ApplicationContextProps>({
changeSetting: (propertyName: string) =>
onApplicationSettingChanged: (propertyName: string) =>
console.log(
`Cannot change setting '${propertyName}'.\nDid you forget to declare an ApplicationProvider ?`
),
settings: {
applicationSettings: {
Copy link
Contributor

Choose a reason for hiding this comment

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

we have an instance of a concrete settings structure here, along with a concrete implementation of an applicationSettingsSchema.

I think maybe its ok to keep the themeMode, as we are using it in the SaltProvider, but the rest don't belong in the library code, they would be provided by client code - i.e in the application that is using the ApplicationContext Provider.

Even with themeMode, we probably shouldn't assume that an app will support light and dark.

Sometimes it makes sense to provide default value(s) for the context object managed by a Provider. I don't think it does here.

themeMode: "light",
dateFormatPattern: "dd MMMM yyyy",
region: "apac",
greyscale: false,
},
applicationSettingsSchema: {
properties: [
{
name: "themeMode",
label: "Mode",
values: ["light", "dark"],
defaultValue: "light",
type: "string",
},
{
name: "dateFormatPattern",
label: "Date Formatting",
values: ["dd/mm/yyyy", "mm/dd/yyyy", "dd MMMM yyyy"],
defaultValue: "dd/mm/yyyy",
type: "string",
},
{
name: "region",
label: "Region",
values: [
{ value: "us", label: "US" },
{ value: "apac", label: "Asia Pacific" },
{ value: "emea", label: "Europe, Middle East & Africa" },
],
defaultValue: "apac",
type: "string",
},
{
name: "greyscale",
label: "Greyscale",
defaultValue: false,
type: "boolean",
},
],
},
user: Guest,
});
46 changes: 36 additions & 10 deletions vuu-ui/packages/vuu-shell/src/application-provider/ApplicationProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,26 +18,37 @@ export interface ApplicationProviderProps

export const ApplicationProvider = ({
children,
settings: settingsProp,
applicationSettings: settingsProp,
applicationSettingsSchema,
user,
}: ApplicationProviderProps): ReactElement => {
const context = useContext(ApplicationContext);
const [settings, setSettings] = useState(settingsProp ?? context.settings);
const [applicationSettings, setSettings] = useState(
settingsProp ?? context.applicationSettings
);

const changeSetting = useCallback((propertyName: string, value: unknown) => {
setSettings((s) => ({ ...s, [propertyName]: value }));
}, []);
const onApplicationSettingChanged = useCallback(
(propertyName: string, value: unknown) => {
setSettings((s) => ({ ...s, [propertyName]: value }));
},
[]
);

return (
<ApplicationContext.Provider
value={{
...context,
changeSetting,
settings,
onApplicationSettingChanged,
applicationSettings,
applicationSettingsSchema,
user: user ?? context.user,
}}
>
<SaltProvider theme="vuu-theme" density="high" mode={settings.themeMode}>
<SaltProvider
theme="vuu-theme"
density="high"
mode={applicationSettings.themeMode}
>
{children}
</SaltProvider>
</ApplicationContext.Provider>
Expand All @@ -49,7 +60,22 @@ export const useApplicationUser = () => {
return user;
};

//Setter method (only used within the shell)
export const useApplicationSettings = () => {
const { changeSetting, settings } = useContext(ApplicationContext);
return { changeSetting, settings };
const {
onApplicationSettingChanged,
applicationSettings,
applicationSettingsSchema,
} = useContext(ApplicationContext);
return {
onApplicationSettingChanged,
applicationSettings,
applicationSettingsSchema,
};
};

//Getter method (read only access to applicationSetting)
export const useApplicationSetting = () => {
const { applicationSettings } = useContext(ApplicationContext);
return { applicationSettings };
};
205 changes: 10 additions & 195 deletions vuu-ui/packages/vuu-shell/src/application-settings/ApplicationSettingsPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,211 +1,28 @@
import { queryClosest } from "@finos/vuu-utils";
import {
Dropdown,
DropdownProps,
FormField,
FormFieldLabel,
Input,
Option,
Switch,
ToggleButton,
ToggleButtonGroup,
ToggleButtonGroupProps,
} from "@salt-ds/core";
import { useComponentCssInjection } from "@salt-ds/styles";
import { useWindow } from "@salt-ds/window";
import {
FormEventHandler,
HTMLAttributes,
SyntheticEvent,
useCallback,
} from "react";
import { HTMLAttributes } from "react";
import { SettingsForm, SettingsSchema } from "./SettingsForm";

Check failure on line 4 in vuu-ui/packages/vuu-shell/src/application-settings/ApplicationSettingsPanel.tsx

View workflow job for this annotation

GitHub Actions / lint-and-typecheck 

'SettingsSchema' is defined but never used. Allowed unused vars must match /^_$/u
Copy link
Contributor

Choose a reason for hiding this comment

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

SettingsSchema can be removed - per the lint error below, its not being used.


import applicationSettingsPanelCss from "./ApplicationSettingsPanel.css";

export type Option<T> = { label: string; value: T };
// Schema type definitions
export const isOption = (
value: Option<number | string> | number | string
): value is Option<number | string> =>
typeof value === "object" && "label" in value && "label" in value;

export interface BaseProperty {
name: string;
label: string;
}

export interface StringProperty extends BaseProperty {
values?: string[] | Option<string>[];
defaultValue?: string;
type: "string";
}
export interface NumericProperty extends BaseProperty {
values?: number[] | Option<number>[];
defaultValue?: number;
type: "number";
}
export interface BooleanProperty extends BaseProperty {
defaultValue?: boolean;
type: "boolean";
}

export type SettingsProperty =
| StringProperty
| NumericProperty
| BooleanProperty;

export const isBooleanProperty = (
property: SettingsProperty
): property is BooleanProperty => property.type === "boolean";

export const isStringOrNumber = (value: unknown): value is string | number =>
typeof value === "string" || typeof value === "number";

export interface SettingsSchema {
properties: SettingsProperty[];
}

const getValueAndLabel = (value: string | number | Option<string | number>) =>
isOption(value) ? [value.value, value.label] : [value, value];

// Determine the form control type to be displayed
export function getFormControl(
property: SettingsProperty,
changeHandler: FormEventHandler,
selectHandler: DropdownProps["onSelectionChange"],
currentValue?: string | boolean | number
) {
if (isBooleanProperty(property)) {
const checked =
typeof currentValue === "boolean"
? currentValue
: property.defaultValue ?? false;

return (
<Switch
checked={checked}
label={property.label}
onChange={changeHandler}
></Switch>
);
}
// Toggle Box for 1 or 2 values
if (Array.isArray(property.values)) {
if (property.values.length <= 2) {
return (
<ToggleButtonGroup
value={currentValue as ToggleButtonGroupProps["value"]}
onChange={changeHandler}
>
{property.values.map((valueOrOption) => {
const [value, label] = getValueAndLabel(valueOrOption);
return (
<ToggleButton key={value} value={value}>
{label}
</ToggleButton>
);
})}
</ToggleButtonGroup>
);
} else if (property.values.length > 2) {
return (
<Dropdown
value={currentValue as DropdownProps["value"]}
onSelectionChange={selectHandler}
>
{property.values.map((valueOrOption) => {
const [value, label] = getValueAndLabel(valueOrOption);
return (
<Option value={value} key={value} data-field={property.name}>
{label}
</Option>
);
})}
</Dropdown>
);
}
} else {
const value = isStringOrNumber(currentValue)
? currentValue
: isStringOrNumber(property.defaultValue)
? property.defaultValue
: "";
return <Input value={value} />;
}
}
import { useApplicationSettings } from "../application-provider";

// Props for Panel
export interface ApplicatonSettingsPanelProps

Check failure on line 10 in vuu-ui/packages/vuu-shell/src/application-settings/ApplicationSettingsPanel.tsx

View workflow job for this annotation

GitHub Actions / lint-and-typecheck 

An interface declaring no members is equivalent to its supertype
Copy link
Contributor

Choose a reason for hiding this comment

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

because ApplicationSettingsPanel no longer implements any props of its own (just inherits from HTMLDivEl;ement), no need to specify a type for the props, just use HTMLAttributes<HTMLDivElement> directly as the type.

If you DID want to declare a type, you would use type rather than interface, so

export  type ApplicatonSettingsPanelProps = HTMLAttributes<HTMLDivElement>;

this is just declaring an alias that doesn't add any new props.

extends HTMLAttributes<HTMLDivElement> {
applicationSettingsSchema: SettingsSchema;
applicationSettings: Record<string, string | number | boolean>;
onApplicationSettingChanged: (
propertyName: string,
value: string | number | boolean
) => void;
}

// Generates application settings form component
export const SettingsForm = ({
applicationSettingsSchema,
applicationSettings,
onApplicationSettingChanged,
}: ApplicatonSettingsPanelProps) => {
const getFieldNameFromEventTarget = (evt: SyntheticEvent) => {
const fieldElement = queryClosest(evt.target, "[data-field]");
if (fieldElement && fieldElement.dataset.field) {
return fieldElement.dataset.field;
} else {
throw Error("data-field attribute not defined");
}
};

// Change Handler for toggle and input buttons
const changeHandler = useCallback<FormEventHandler>(
(event) => {
const fieldName = getFieldNameFromEventTarget(event);
const { checked, value } = event.target as HTMLInputElement;
onApplicationSettingChanged(fieldName, checked ?? value);
},
[onApplicationSettingChanged]
);

// Change handler for selection form controls
const selectHandler = useCallback(
(event: SyntheticEvent, [selected]: string[]) => {
const fieldName = getFieldNameFromEventTarget(event);
onApplicationSettingChanged(fieldName, selected);
},
[onApplicationSettingChanged]
);

return (
<div>
{applicationSettingsSchema.properties.map((property) => (
<FormField data-field={property.name} key={property.name}>
<FormFieldLabel>{property.label}</FormFieldLabel>
{getFormControl(
property,
changeHandler,
selectHandler,
applicationSettings[property.name]
)}
</FormField>
))}
</div>
);
};
extends HTMLAttributes<HTMLDivElement> {}

const classBase = "vuuApplicationSettingsPanel";

export const ApplicationSettingsPanel = ({
applicationSettingsSchema,
applicationSettings,
onApplicationSettingChanged,
...htmlAttributes
}: ApplicatonSettingsPanelProps) => {
const targetWindow = useWindow();

const {
onApplicationSettingChanged,
applicationSettings,
applicationSettingsSchema,
} = useApplicationSettings();

useComponentCssInjection({
testId: "vuu-application-settings-panel",
css: applicationSettingsPanelCss,
Expand All @@ -222,5 +39,3 @@
</div>
);
};

export default ApplicationSettingsPanel;
Loading
Loading