Skip to content

Commit

Permalink
wip conditional theme flag
Browse files Browse the repository at this point in the history
render fieldsets based on flag value

fix action list item button padding
  • Loading branch information
jenny-s51 committed Nov 15, 2024
1 parent 30f7c26 commit 97d8687
Show file tree
Hide file tree
Showing 14 changed files with 463 additions and 278 deletions.
31 changes: 31 additions & 0 deletions clients/ui/frontend/src/app/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
Spinner,
Stack,
StackItem,
Switch,
} from '@patternfly/react-core';
import { BarsIcon } from '@patternfly/react-icons';
import ToastNotifications from '~/shared/components/ToastNotifications';
Expand All @@ -26,6 +27,12 @@ import AppRoutes from './AppRoutes';
import { AppContext } from './AppContext';
import { ModelRegistrySelectorContextProvider } from './context/ModelRegistrySelectorContext';

declare global {
export interface Window {
isSwitched?: boolean;
}
}

Check failure on line 34 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Delete `··`

const App: React.FC = () => {
const {
configSettings,
Expand Down Expand Up @@ -77,6 +84,23 @@ const App: React.FC = () => {
// Waiting on the API to finish
const loading = !configLoaded || !userSettings || !configSettings || !contextValue;

const [isChecked, setIsChecked] = React.useState<boolean>(window.isSwitched ?? false);

Check failure on line 87 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

React Hook "React.useState" is called conditionally. React Hooks must be called in the exact same order in every component render

// Sync window.isSwitched with local state when it changes
React.useEffect(() => {

Check failure on line 90 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

React Hook "React.useEffect" is called conditionally. React Hooks must be called in the exact same order in every component render. Did you accidentally call a React Hook after an early return?
window.isSwitched = isChecked;

Check failure on line 91 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Delete `·`
document.documentElement.classList.toggle('mui-theme', isChecked);

const event = new CustomEvent('isSwitchedChanged', {
detail: isChecked,
});
window.dispatchEvent(event);

Check failure on line 98 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Replace `⏎··},·[isChecked]);·` with `··},·[isChecked]);`
}, [isChecked]);

const handleChange = (_event: React.FormEvent<HTMLInputElement>, checked: boolean) => {
setIsChecked(checked);

Check failure on line 102 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Delete `·`
};
const masthead = (
<Masthead>
<MastheadMain>
Expand All @@ -95,6 +119,13 @@ const App: React.FC = () => {
</MastheadMain>

<MastheadContent>
<Switch

Check failure on line 122 in clients/ui/frontend/src/app/App.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Insert `··`
id="simple-switch"
label="Toggle MUI Theme"
isChecked={isChecked}
onChange={handleChange}
ouiaId="BasicSwitch"
/>
{/* TODO: [Auth-enablement] Add logout and user status once we enable itNavigates to register page from table toolbar */}
</MastheadContent>
</Masthead>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,24 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({

const [isValueExpanded, setIsValueExpanded] = React.useState(false);

const [isSwitched, setIsSwitched] = React.useState<boolean>(window.isSwitched ?? false);

// Listen for changes to `window.isSwitched`
React.useEffect(() => {
const handleSwitchChange = (event: Event) => {
const customEvent = event as CustomEvent;

Check failure on line 57 in clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Do not use any type assertions
setIsSwitched(customEvent.detail); // Update state with the new value

Check failure on line 58 in clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelPropertiesTableRow.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Delete `·`
};

// Add event listener for custom event
window.addEventListener('isSwitchedChanged', handleSwitchChange);

// Cleanup the event listener when the component unmounts
return () => {
window.removeEventListener('isSwitchedChanged', handleSwitchChange);
};
}, []); // Empty dependency array means this effect runs only once

let keyValidationError: string | null = null;
if (unsavedKey !== key && allExistingKeys.includes(unsavedKey)) {
keyValidationError = 'Key must not match an existing property key or label';
Expand Down Expand Up @@ -90,31 +108,45 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({
setIsEditing(false);
};

const propertyKeyInput = (
<TextInput
data-testid={isAddRow ? `add-property-key-input` : `edit-property-${key}-key-input`}
aria-label={
isAddRow ? 'Key input for new property' : `Key input for editing property with key ${key}`
}
isRequired
type="text"
value={unsavedKey}
onChange={(_event, str) => setUnsavedKey(str)}
validated={keyValidationError ? 'error' : 'default'}
/>
);

const propertyValueInput = (
<TextInput
data-testid={isAddRow ? `add-property-value-input` : `edit-property-${key}-value-input`}
aria-label={
isAddRow
? 'Value input for new property'
: `Value input for editing property with key ${key}`
}
isRequired
type="text"
value={unsavedValue}
onChange={(_event, str) => setUnsavedValue(str)}
/>
);

return (
<Tr>
<Td dataLabel="Key" width={45} modifier="breakWord">
{isEditing ? (
<>
<FormFieldset
className="tr-fieldset-wrapper"
component={
<TextInput
data-testid={
isAddRow ? `add-property-key-input` : `edit-property-${key}-key-input`
}
aria-label={
isAddRow
? 'Key input for new property'
: `Key input for editing property with key ${key}`
}
isRequired
type="text"
value={unsavedKey}
onChange={(_event, str) => setUnsavedKey(str)}
validated={keyValidationError ? 'error' : 'default'}
/>
}
/>
{window.isSwitched ? (
<FormFieldset className="tr-fieldset-wrapper" component={propertyKeyInput} />
) : (
propertyKeyInput
)}

{keyValidationError && (
<FormHelperText>
Expand All @@ -130,25 +162,11 @@ const ModelPropertiesTableRow: React.FC<ModelPropertiesTableRowProps> = ({
</Td>
<Td dataLabel="Value" width={45} modifier="breakWord">
{isEditing ? (
<FormFieldset
className="tr-fieldset-wrapper"
component={
<TextInput
data-testid={
isAddRow ? `add-property-value-input` : `edit-property-${key}-value-input`
}
aria-label={
isAddRow
? 'Value input for new property'
: `Value input for editing property with key ${key}`
}
isRequired
type="text"
value={unsavedValue}
onChange={(_event, str) => setUnsavedValue(str)}
/>
}
/>
isSwitched ? (
<FormFieldset className="tr-fieldset-wrapper" component={propertyValueInput} />
) : (
propertyValueInput
)
) : (
<ExpandableSection
variant="truncate"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
DropdownList,
MenuToggle,
MenuToggleElement,
SearchInput,
TextInput,
ToolbarContent,
ToolbarFilter,
Expand Down Expand Up @@ -53,6 +54,19 @@ const ModelVersionListView: React.FC<ModelVersionListViewProps> = ({
const [searchType, setSearchType] = React.useState<SearchType>(SearchType.KEYWORD);
const [search, setSearch] = React.useState('');

const [isSwitched, setIsSwitched] = React.useState<boolean>(window.isSwitched ?? false);

React.useEffect(() => {

Check failure on line 60 in clients/ui/frontend/src/app/pages/modelRegistry/screens/ModelVersions/ModelVersionListView.tsx

View workflow job for this annotation

GitHub Actions / test-and-build

Delete `⏎`
const handleSwitchChange = () => {
setIsSwitched(window.isSwitched ?? false);
};
window.addEventListener('isSwitchedChanged', handleSwitchChange);
return () => {
window.removeEventListener('isSwitchedChanged', handleSwitchChange);
};
}, []);

const searchTypes = [SearchType.KEYWORD, SearchType.AUTHOR];

const [isArchivedModelVersionKebabOpen, setIsArchivedModelVersionKebabOpen] =
Expand Down Expand Up @@ -148,22 +162,35 @@ const ModelVersionListView: React.FC<ModelVersionListViewProps> = ({
/>
</ToolbarFilter>
<ToolbarItem>
<FormFieldset
className="toolbar-fieldset-wrapper"
component={
<TextInput
value={search}
type="text"
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
style={{ minWidth: '200px' }}
data-testid="model-versions-table-search"
aria-label="Search"
/>
}
field={`Find by ${searchType.toLowerCase()}`}
/>
{isSwitched ? (
<FormFieldset
className="toolbar-fieldset-wrapper"
component={
<TextInput
value={search}
type="text"
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
style={{ minWidth: '200px' }}
data-testid="model-versions-table-search"
aria-label="Search"
/>
}
field={`Find by ${searchType.toLowerCase()}`}
/>
) : (
<SearchInput
placeholder={`Find by ${searchType.toLowerCase()}`}
value={search}
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
onClear={() => setSearch('')}
style={{ minWidth: '200px' }}
data-testid="model-versions-table-search"
/>
)}
</ToolbarItem>
</ToolbarGroup>
</ToolbarToggleGroup>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from 'react';
import {
SearchInput,
TextInput,
ToolbarContent,
ToolbarFilter,
Expand Down Expand Up @@ -31,6 +32,18 @@ const ModelVersionsArchiveListView: React.FC<ModelVersionsArchiveListViewProps>

const searchTypes = [SearchType.KEYWORD, SearchType.AUTHOR];

const [isSwitched, setIsSwitched] = React.useState<boolean>(window.isSwitched ?? false);

React.useEffect(() => {
const handleSwitchChange = () => {
setIsSwitched(window.isSwitched ?? false);
};
window.addEventListener('isSwitchedChanged', handleSwitchChange);
return () => {
window.removeEventListener('isSwitchedChanged', handleSwitchChange);
};
}, []);

const filteredModelVersions = filterModelVersions(unfilteredmodelVersions, search, searchType);

if (unfilteredmodelVersions.length === 0) {
Expand Down Expand Up @@ -74,22 +87,35 @@ const ModelVersionsArchiveListView: React.FC<ModelVersionsArchiveListViewProps>
/>
</ToolbarFilter>
<ToolbarItem>
<FormFieldset
className="toolbar-fieldset-wrapper"
component={
<TextInput
value={search}
type="text"
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
style={{ minWidth: '200px' }}
data-testid="model-versions-archive-table-search"
aria-label="Search"
/>
}
field={`Find by ${searchType.toLowerCase()}`}
/>
{isSwitched ? (
<FormFieldset
className="toolbar-fieldset-wrapper"
component={
<TextInput
value={search}
type="text"
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
style={{ minWidth: '200px' }}
data-testid="model-versions-archive-table-search"
aria-label="Search"
/>
}
field={`Find by ${searchType.toLowerCase()}`}
/>
) : (
<SearchInput
placeholder={`Find by ${searchType.toLowerCase()}`}
value={search}
onChange={(_, searchValue) => {
setSearch(searchValue);
}}
onClear={() => setSearch('')}
style={{ minWidth: '200px' }}
data-testid="model-versions-archive-table-search"
/>
)}
</ToolbarItem>
</ToolbarGroup>
</ToolbarToggleGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,20 @@ type PrefilledModelRegistryFieldProps = {
mrName?: string;
};

const PrefilledModelRegistryField: React.FC<PrefilledModelRegistryFieldProps> = ({ mrName }) => (
<FormGroup className="form-group-disabled" label="Model registry" isRequired fieldId="mr-name">
<FormFieldset
component={
<TextInput isDisabled isRequired type="text" id="mr-name" name="mr-name" value={mrName} />
}
field="Model Registry"
/>
</FormGroup>
);
const PrefilledModelRegistryField: React.FC<PrefilledModelRegistryFieldProps> = ({ mrName }) => {
const mrNameInput = (
<TextInput isDisabled isRequired type="text" id="mr-name" name="mr-name" value={mrName} />
);

return (
<FormGroup className="form-group-disabled" label="Model registry" isRequired fieldId="mr-name">
{window.isSwitched ? (
<FormFieldset component={mrNameInput} field="Model Registry" />
) : (
mrNameInput
)}
</FormGroup>
);
};

export default PrefilledModelRegistryField;
Loading

0 comments on commit 97d8687

Please sign in to comment.