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.

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

Converting GlobalFilter to Jotai #2955

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
73 changes: 73 additions & 0 deletions src/@types/types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
* Only use as placeholder
*/
export type AnyObject = {
[key: string]: any;

Check warning on line 31 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
};

export type NavItemPermission<T extends keyof VisibilityFunctions = 'isOrgAdmin'> = {
Expand Down Expand Up @@ -68,10 +68,10 @@

declare global {
interface Window {
hj: any;

Check warning on line 71 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
pendo?: {
updateOptions: (...args: any[]) => void;

Check warning on line 73 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
initialize: (config: Record<string, any>) => void;

Check warning on line 74 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
flushNow: () => void;
setGuidesDisabled: (disabled: boolean) => void;
startGuides: () => void;
Expand Down Expand Up @@ -135,7 +135,7 @@
manifestLocation: string;
dynamic?: boolean;
exact?: boolean;
props?: any;

Check warning on line 138 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
};

export type SupportCaseConfig = {
Expand Down Expand Up @@ -190,6 +190,79 @@

export type FlagTagsFilter = Record<string, Record<string, boolean | GroupItem>>;

export type GlobalFilterWorkloads = {
justinorringer marked this conversation as resolved.
Show resolved Hide resolved
selected?: boolean;
page?: number;
perPage?: number;
isLoaded: boolean;
name: 'Workloads';
noFilter?: true;
tags?: { tag?: CommonTag; count: number }[];
items?: any[];

Check warning on line 201 in src/@types/types.d.ts

View workflow job for this annotation

GitHub Actions / lint

Unexpected any. Specify a different type
count?: number;
total?: number;
};

export type CommonTag = {
key?: string;
namespace?: string;
value?: string | number | boolean;
};

export type CommonSelectedTag = CommonTag & {
id: string;
cells: [string, string, string];
selected?: boolean;
};

export type SID = {
id?: string;
name?: string;
tags?: {
tag: CommonTag;
}[];
};

export type GlobalFilterSIDs = {
isLoaded: boolean;
total?: number;
count?: number;
page?: number;
perPage?: number;
items?: SID[];
};

export type GlobalFilterTag = {
id?: string;
name?: string;
tags?: {
tag: CommonTag;
}[];
};

export type GlobalFilterTags = {
isLoaded: boolean;
items: GlobalFilterTag[];
total?: number;
count?: number;
page?: number;
perPage?: number;
};

export type TagRegisteredWith = Array<
'insights' | 'yupana' | 'puptoo' | 'rhsm-conduit' | 'cloud-connector' | '!yupana' | '!puptoo' | '!rhsm-conduit' | '!cloud-connector'
>;

export type GlobalFilterState = {
tags: GlobalFilterTags;
globalFilterRemoved?: boolean;
workloads: GlobalFilterWorkloads;
sid: GlobalFilterSIDs;
selectedTags?: FlagTagsFilter;
globalFilterHidden: boolean;
scope?: TagRegisteredWith[number];
};

export type ChromeNavItemProps = {
isHidden?: boolean;
ignoreCase?: boolean;
Expand Down
15 changes: 6 additions & 9 deletions src/chrome/create-chrome.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import { createFetchPermissionsWatcher } from '../auth/fetchPermissions';
import { AddChromeWsEventListener, AppNavigationCB, ChromeAPI, GenericCB } from '@redhat-cloud-services/types';
import { Store } from 'redux';
import { AnalyticsBrowser } from '@segment/analytics-next';
import get from 'lodash/get';
import Cookies from 'js-cookie';

import { globalFilterScope, removeGlobalFilter, toggleGlobalFilter } from '../redux/actions';
import { globalFilterScope, removeGlobalFilter, toggleGlobalFilter } from '../state/actions/globalFilterActions';
import { ITLess, getEnv, getEnvDetails, isProd, updateDocumentTitle } from '../utils/common';
import { createSupportCase } from '../utils/createCase';
import debugFunctions from '../utils/debugFunctions';
Expand All @@ -14,7 +13,6 @@ import { PUBLIC_EVENTS } from '../utils/consts';
import { middlewareListener } from '../redux/redux-config';
import { clearAnsibleTrialFlag, isAnsibleTrialFlagActive, setAnsibleTrialFlag } from '../utils/isAnsibleTrialFlagActive';
import chromeHistory from '../utils/chromeHistory';
import { ReduxState } from '../redux/store';
import { FlagTagsFilter } from '../@types/types';
import useBundle, { bundleMapping, getUrl } from '../hooks/useBundle';
import { warnDuplicatePkg } from './warnDuplicatePackages';
Expand All @@ -32,7 +30,7 @@ import { appActionAtom, pageObjectIdAtom } from '../state/atoms/pageAtom';

export type CreateChromeContextConfig = {
useGlobalFilter: (callback: (selectedTags?: FlagTagsFilter) => any) => ReturnType<typeof callback>;
store: Store<ReduxState>;
store: typeof chromeStore;
setPageMetadata: (pageOptions: any) => any;
analytics: AnalyticsBrowser;
quickstartsAPI: ChromeAPI['quickStarts'];
Expand Down Expand Up @@ -61,16 +59,15 @@ export const createChromeContext = ({
}: CreateChromeContextConfig): ChromeAPI => {
const fetchPermissions = createFetchPermissionsWatcher(chromeAuth.getUser);
const visibilityFunctions = getVisibilityFunctions();
const dispatch = store.dispatch;
const actions = {
appAction: (action: string) => chromeStore.set(appActionAtom, action),
appObjectId: (objectId: string) => chromeStore.set(pageObjectIdAtom, objectId),
appNavClick: (item: string) => chromeStore.set(activeAppAtom, item),
globalFilterScope: (scope: string) => dispatch(globalFilterScope(scope)),
globalFilterScope: (scope: string) => globalFilterScope(scope),
registerModule: (module: string, manifest?: string) => registerModule({ module, manifest }),
removeGlobalFilter: (isHidden: boolean) => {
console.error('`removeGlobalFilter` is deprecated. Use `hideGlobalFilter` instead.');
return dispatch(removeGlobalFilter(isHidden));
return removeGlobalFilter(isHidden);
},
};

Expand All @@ -86,7 +83,7 @@ export const createChromeContext = ({
const [listener, selector] = PUBLIC_EVENTS[type];
if (typeof selector === 'string') {
(callback as GenericCB)({
data: get(store.getState(), selector) || {},
data: get(store, selector) || {},
});
}
if (typeof listener === 'function') {
Expand Down Expand Up @@ -146,7 +143,7 @@ export const createChromeContext = ({
},
identifyApp,
hideGlobalFilter: (isHidden: boolean) => {
dispatch(toggleGlobalFilter(isHidden));
toggleGlobalFilter(isHidden);
},
isBeta: () => isPreview,
isChrome2: true,
Expand Down
2 changes: 1 addition & 1 deletion src/components/ChromeRoute/ChromeRoute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { ScalprumComponent } from '@scalprum/react-core';
import React, { memo, useContext, useEffect, useState } from 'react';
import LoadingFallback from '../../utils/loading-fallback';
import { batch, useDispatch } from 'react-redux';
import { toggleGlobalFilter } from '../../redux/actions';
import { toggleGlobalFilter } from '../../state/actions/globalFilterActions';
import ErrorComponent from '../ErrorComponents/DefaultErrorComponent';
import classNames from 'classnames';
import { HelpTopicContext } from '@patternfly/quickstarts';
Expand Down
71 changes: 28 additions & 43 deletions src/components/GlobalFilter/GlobalFilter.tsx
Original file line number Diff line number Diff line change
@@ -1,51 +1,46 @@
import React, { useCallback, useContext, useEffect, useMemo, useState } from 'react';
import { batch, shallowEqual, useDispatch, useSelector } from 'react-redux';
import { useTagsFilter } from '@redhat-cloud-services/frontend-components/FilterHooks';
import debounce from 'lodash/debounce';
import { fetchAllSIDs, fetchAllTags, fetchAllWorkloads, globalFilterChange } from '../../redux/actions';
import { fetchAllSIDs, fetchAllTags, fetchAllWorkloads, globalFilterChange } from '../../state/actions/globalFilterActions';
import { generateFilter } from './globalFilterApi';
import { useLocation, useNavigate } from 'react-router-dom';
import { GlobalFilterDropdown, GlobalFilterDropdownProps } from './GlobalFilterMenu';
import { storeFilter } from './filterApi';
import { GlobalFilterTag, GlobalFilterWorkloads, ReduxState, SID } from '../../redux/store';
import { GlobalFilterTag, GlobalFilterWorkloads, SID } from '../../@types/types';
import { FlagTagsFilter } from '../../@types/types';
import { isGlobalFilterAllowed } from '../../utils/common';
import InternalChromeContext from '../../utils/internalChromeContext';
import ChromeAuthContext from '../../auth/ChromeAuthContext';
import { useAtomValue } from 'jotai';
import { activeModuleAtom } from '../../state/atoms/activeModuleAtom';
import { globalFilterReducerAtom } from '../../state/atoms/globalFilterAtom';
import useGlobalFilterState from '../../hooks/useGlobalFilterState';

const useLoadTags = (hasAccess = false) => {
const navigate = useNavigate();
const registeredWith = useSelector(({ globalFilter: { scope } }: ReduxState) => scope);
const globalFilterState = useAtomValue(globalFilterReducerAtom);
const registeredWith = globalFilterState.scope;
const activeModule = useAtomValue(activeModuleAtom);
const isDisabled = useSelector(({ globalFilter: { globalFilterHidden } }: ReduxState) => globalFilterHidden || !activeModule);
const dispatch = useDispatch();
const isDisabled = globalFilterState.globalFilterHidden || !activeModule;
return useCallback(
debounce((activeTags: any, search: any) => {
storeFilter(activeTags, hasAccess && !isDisabled, navigate);
batch(() => {
dispatch(
fetchAllTags({
registeredWith,
activeTags,
search,
})
);
dispatch(
fetchAllSIDs({
registeredWith,
activeTags,
search,
})
);
dispatch(
fetchAllWorkloads({
registeredWith,
activeTags,
search,
})
);
// will removing the batch here cause problems? they are all fetches
justinorringer marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/pmndrs/jotai/discussions/2416
fetchAllTags({
registeredWith,
activeTags,
search,
});
fetchAllSIDs({
registeredWith,
activeTags,
search,
});
fetchAllWorkloads({
registeredWith,
activeTags,
search,
});
}, 600),
[registeredWith, hasAccess]
Expand All @@ -54,18 +49,7 @@ const useLoadTags = (hasAccess = false) => {

const GlobalFilter = ({ hasAccess }: { hasAccess: boolean }) => {
const [isOpen, setIsOpen] = useState(false);
const dispatch = useDispatch();
const isLoaded = useSelector(({ globalFilter: { tags, sid, workloads } }: ReduxState) => tags.isLoaded && sid.isLoaded && workloads.isLoaded);
const { count, total, tags, sid, workloads } = useSelector(
({ globalFilter: { tags, sid, workloads } }: ReduxState) => ({
count: (tags.count || 0) + (sid.count || 0) + (workloads.count || 0),
total: (tags.total || 0) + (sid.total || 0) + (workloads.total || 0),
tags: tags.items || [],
sid: sid.items || [],
workloads,
}),
shallowEqual
);
const { count, total, tags, sid, workloads, isLoaded } = useGlobalFilterState();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a hook for this, may be unnecessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

When consolidating state variables, just be careful you are properly memorizing the state output. To prevent extra re-renders.

I don't see an issue with using the jotai atoms directly.


const { filter, chips, selectedTags, setValue, filterTagsBy } = (
useTagsFilter as unknown as (
Expand Down Expand Up @@ -98,7 +82,7 @@ const GlobalFilter = ({ hasAccess }: { hasAccess: boolean }) => {

const loadTags = useLoadTags(hasAccess);
const selectTags = useCallback(
debounce((selectedTags: FlagTagsFilter) => dispatch(globalFilterChange(selectedTags)), 600),
debounce((selectedTags: FlagTagsFilter) => globalFilterChange(selectedTags), 600),
[globalFilterChange]
);

Expand Down Expand Up @@ -129,15 +113,16 @@ const GlobalFilter = ({ hasAccess }: { hasAccess: boolean }) => {

const GlobalFilterWrapper = () => {
const [hasAccess, setHasAccess] = useState(false);
const globalFilterRemoved = useSelector(({ globalFilter: { globalFilterRemoved } }: ReduxState) => globalFilterRemoved);
const globalFilterState = useAtomValue(globalFilterReducerAtom);
const globalFilterRemoved = globalFilterState.globalFilterRemoved;
const chromeAuth = useContext(ChromeAuthContext);
const { pathname } = useLocation();
const { getUserPermissions } = useContext(InternalChromeContext);

// FIXME: Clean up the global filter display flag
const isLanding = pathname === '/';
const isAllowed = isGlobalFilterAllowed();
const globalFilterHidden = useSelector(({ globalFilter: { globalFilterHidden } }: ReduxState) => globalFilterHidden);
const globalFilterHidden = globalFilterState.globalFilterHidden;
const activeModule = useAtomValue(activeModuleAtom);
const isDisabled = globalFilterHidden || !activeModule;
const isGlobalFilterEnabled = useMemo(() => {
Expand Down
24 changes: 11 additions & 13 deletions src/components/GlobalFilter/GlobalFilterMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { useIntl } from 'react-intl';
import messages from '../../locales/Messages';

import './global-filter-menu.scss';
import { useDispatch, useSelector } from 'react-redux';
import { useAtomValue } from 'jotai';
import { globalFilterReducerAtom } from '../../state/atoms/globalFilterAtom';
import { Button } from '@patternfly/react-core/dist/dynamic/components/Button';
import { Chip, ChipGroup } from '@patternfly/react-core/dist/dynamic/components/Chip';
import { Divider } from '@patternfly/react-core/dist/dynamic/components/Divider';
Expand All @@ -14,10 +15,9 @@ import { Split, SplitItem } from '@patternfly/react-core/dist/dynamic/layouts/Sp
import { Tooltip } from '@patternfly/react-core/dist/dynamic/components/Tooltip';
import TagsModal from './TagsModal';
import { FilterMenuItemOnChange } from '@redhat-cloud-services/frontend-components/ConditionalFilter/groupFilterConstants';
import { CommonSelectedTag, ReduxState } from '../../redux/store';
import { updateSelected } from './globalFilterApi';
import { fetchAllTags } from '../../redux/actions';
import { FlagTagsFilter } from '../../@types/types';
import { fetchAllTags } from '../../state/actions/globalFilterActions';
import { CommonSelectedTag, FlagTagsFilter } from '../../@types/types';
import ChromeAuthContext from '../../auth/ChromeAuthContext';
import GroupFilterInputGroup from './GroupFilterInputGroup';

Expand Down Expand Up @@ -89,10 +89,10 @@ export const GlobalFilterDropdown: React.FunctionComponent<GlobalFilterDropdownP
* We are unable to test it in any local development environment
* */
const hotjarEventEmitter = typeof window.hj === 'function' ? window.hj : () => undefined;
const registeredWith = useSelector(({ globalFilter: { scope } }: ReduxState) => scope);
const globalFilterState = useAtomValue(globalFilterReducerAtom);
const registeredWith = globalFilterState.scope;
const auth = useContext(ChromeAuthContext);
const intl = useIntl();
const dispatch = useDispatch();
const GroupFilterWrapper = useMemo(
() => (!allowed || isDisabled ? Tooltip : ({ children }: { children: any }) => <Fragment>{children}</Fragment>),
[allowed, isDisabled]
Expand Down Expand Up @@ -161,13 +161,11 @@ export const GlobalFilterDropdown: React.FunctionComponent<GlobalFilterDropdownP
selectedTags={selectedTags}
toggleModal={(isSubmit) => {
if (!isSubmit) {
dispatch(
fetchAllTags({
registeredWith: registeredWith as 'insights',
activeTags: selectedTags,
search: filterTagsBy,
})
);
fetchAllTags({
registeredWith: registeredWith as 'insights',
activeTags: selectedTags,
search: filterTagsBy,
});
}
hotjarEventEmitter('event', 'global_filter_bulk_action');
setIsOpen(false);
Expand Down
18 changes: 10 additions & 8 deletions src/components/GlobalFilter/TagsModal.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { shallowEqual, useDispatch, useSelector } from 'react-redux';
import { TagModal } from '@redhat-cloud-services/frontend-components/TagModal';
import { fetchAllSIDs, fetchAllTags } from '../../redux/actions';
import { fetchAllSIDs, fetchAllTags } from '../../state/actions/globalFilterActions';
import debounce from 'lodash/debounce';
import flatMap from 'lodash/flatMap';
import { useIntl } from 'react-intl';
Expand All @@ -10,8 +9,10 @@ import { Action } from 'redux';
import { TableWithFilterPagination } from '@redhat-cloud-services/frontend-components/TagModal/TableWithFilter';
import { OnSelectRow, OnUpdateData } from '@redhat-cloud-services/frontend-components/TagModal/TagModal';
import messages from '../../locales/Messages';
import { CommonSelectedTag, CommonTag, GlobalFilterTag, ReduxState, SID } from '../../redux/store';
import { CommonSelectedTag, CommonTag, GlobalFilterTag, SID } from '../../@types/types';
import { FlagTagsFilter } from '../../@types/types';
import { useAtomValue } from 'jotai';
import { globalFilterStateAtom } from '../../state/atoms/globalFilterAtom';

export type TagsModalProps = {
isOpen?: boolean;
Expand All @@ -25,11 +26,12 @@ export type IDMapper = (tag: CommonTag) => string;
export type CellsMapper = (tag: CommonTag) => (string | number | boolean | undefined)[];
export type DebounceCallback = (filters?: TagFilterOptions, pagination?: TagPagination) => Action;

export const useMetaSelector = (key: 'tags' | 'workloads' | 'sid') =>
useSelector<ReduxState, [boolean | unknown, number, number, number]>(({ globalFilter }) => {
const selected = globalFilter[key];
return [selected?.isLoaded, selected?.total || 0, selected?.page || 1, selected?.perPage || 10];
}, shallowEqual);
export const useMetaSelector = (key: 'tags' | 'workloads' | 'sid') => {
const globalFilterState = useAtomValue(globalFilterStateAtom);
const selected = globalFilterState[key];

return [selected?.isLoaded, selected?.total || 0, selected?.page || 1, selected?.perPage || 10];
};

const usePagination = (loaded: boolean | unknown, perPage?: number, page?: number, count?: number) => {
return useMemo(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/components/Header/MastheadMenuToggle.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { MastheadToggle } from '@patternfly/react-core/dist/dynamic/components/M
import { PageToggleButton } from '@patternfly/react-core/dist/dynamic/components/Page';
import BarsIcon from '@patternfly/react-icons/dist/dynamic/icons/bars-icon';
import { useDispatch } from 'react-redux';
import { onToggle } from '../../redux/actions';
import { onToggle } from '../../state/actions/globalFilterActions';

const MastheadMenuToggle = ({
isNavOpen,
Expand Down
Loading
Loading