Skip to content

Commit

Permalink
fix(RHIF-232): flickering inventory table is fixed, api reqs are redu…
Browse files Browse the repository at this point in the history
…ced to 1

* chore: add ignoreRefresh prop to fix inventoryTable flickering

* fix(RHIF-232): flickering inventory table is fixed, api reqs are reduced to 1
  • Loading branch information
mkholjuraev authored Apr 3, 2023
1 parent cf82190 commit 97af8a3
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/components/InventoryTable/EntityTableToolbar.js
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ const EntityTableToolbar = ({
* @param {*} debounced if debounce function should be used.
*/
const onSetTextFilter = (value, debounced = true) => {
const trimmedValue = value.trim();
const trimmedValue = value?.trim();

const textualFilter = filters?.find(oneFilter => oneFilter.value === TEXT_FILTER);
if (textualFilter) {
Expand Down
52 changes: 29 additions & 23 deletions src/components/InventoryTable/InventoryTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,25 @@ import AccessDenied from '../../Utilities/AccessDenied';
import { loadSystems } from '../../Utilities/sharedFunctions';
import isEqual from 'lodash/isEqual';
import { entitiesLoading } from '../../store/actions';
import cloneDeep from 'lodash/cloneDeep';

/**
* A helper function to store props and to always return the latest state.
* For example, EntityTableToolbar wraps OnRefreshData in a callback, so we need this
* to get the latest props and not the props at the time of when the function is
* being wrapped in callback.
*/
const propsCache = () => {
const inventoryCache = () => {
let cache = {};

const updateProps = (props) => { cache = props; };
const updateProps = (props) => { cache = cloneDeep({ ...cache, props }); };

const getProps = () => cache;
const updateParams = (params) => { cache = cloneDeep({ ...cache, params }); };

return { updateProps, getProps };
const getProps = () => cache.props;
const getParams = () => cache.params;

return { updateProps, updateParams, getProps, getParams };
};

/**
Expand Down Expand Up @@ -71,12 +75,10 @@ const InventoryTable = forwardRef(({ // eslint-disable-line react/display-name
));
const page = useSelector(({ entities: { page: invPage } }) => (
hasItems ? propsPage : (invPage || 1)
)
, shallowEqual);
), shallowEqual);
const perPage = useSelector(({ entities: { perPage: invPerPage } }) => (
hasItems ? propsPerPage : (invPerPage || 50)
)
, shallowEqual);
), shallowEqual);
const total = useSelector(({ entities: { total: invTotal } }) => {
if (hasItems) {
return propsTotal !== undefined ? propsTotal : items?.length;
Expand Down Expand Up @@ -112,7 +114,7 @@ const InventoryTable = forwardRef(({ // eslint-disable-line react/display-name
const dispatch = useDispatch();
const store = useStore();

const cache = useRef(propsCache());
const cache = useRef(inventoryCache());
cache.current.updateProps({
page,
perPage,
Expand All @@ -134,7 +136,7 @@ const InventoryTable = forwardRef(({ // eslint-disable-line react/display-name
const cachedProps = cache.current?.getProps() || {};
const currPerPage = options?.per_page || options?.perPage || cachedProps.perPage;

const params = {
const newParams = {
page: cachedProps.page,
per_page: currPerPage,
items: cachedProps.items,
Expand All @@ -146,25 +148,29 @@ const InventoryTable = forwardRef(({ // eslint-disable-line react/display-name
...options
};

if (onRefresh && !disableOnRefresh) {
dispatch(entitiesLoading());
onRefresh(params, (options) => {
const cachedParams = cache.current.getParams();
if (!isEqual(cachedParams, newParams)) {
cache.current.updateParams(newParams);
if (onRefresh && !disableOnRefresh) {
dispatch(entitiesLoading());
onRefresh(newParams, (options) => {
dispatch(
loadSystems(
{ ...newParams, ...options },
cachedProps.showTags,
cachedProps.getEntities
)
);
});
} else {
dispatch(
loadSystems(
{ ...params, ...options },
newParams,
cachedProps.showTags,
cachedProps.getEntities
)
);
});
} else {
dispatch(
loadSystems(
params,
cachedProps.showTags,
cachedProps.getEntities
)
);
}
}
};

Expand Down
2 changes: 1 addition & 1 deletion src/components/filters/useTextFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export const useTextFilter = ([state, dispatch] = [textFilterState]) => {
onChange: (_e, value) => setValue(value)
}
};
const chip = value.length > 0 ? [{
const chip = value?.length > 0 ? [{
category: 'Display name',
type: TEXTUAL_CHIP,
chips: [
Expand Down
5 changes: 5 additions & 0 deletions src/routes/InventoryTable.js
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,10 @@ const Inventory = ({
Array.isArray(perPage) ? perPage[0] : perPage
));
}

return () => {
dispatch(actions.clearEntitiesAction());
};
}, []);

const calculateSelected = () => selected ? selected.size : 0;
Expand Down Expand Up @@ -236,6 +240,7 @@ const Inventory = ({
onRefresh={onRefresh}
hasCheckbox={writePermissions}
autoRefresh
ignoreRefresh
initialLoading={initialLoading}
tableProps={
(writePermissions && {
Expand Down
1 change: 1 addition & 0 deletions src/store/action-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,3 +67,4 @@ export const CLEAR_FILTERS = 'CLEAR_FILTERS';
export const TOGGLE_TAG_MODAL = 'TOGGLE_TAG_MODAL';
export const CONFIG_CHANGED = 'CONFIG_CHANGED';
export const TOGGLE_DRAWER = 'TOGGLE_INVENTORY_DRAWER';
export const CLEAR_ENTITIES = 'CLEAR_ENTITIES';
8 changes: 7 additions & 1 deletion src/store/actions.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { ACTION_TYPES, CLEAR_NOTIFICATIONS, SET_INVENTORY_FILTER, SET_PAGINATION } from './action-types';
import { ACTION_TYPES, CLEAR_NOTIFICATIONS, SET_INVENTORY_FILTER, SET_PAGINATION,
CLEAR_ENTITIES } from './action-types';
import { hosts, getEntitySystemProfile } from '../api';
export * from './system-issues-actions';
export * from './inventory-actions';
Expand Down Expand Up @@ -77,3 +78,8 @@ export const editAnsibleHost = (id, value, origValue) => ({
}
}
});

export const clearEntitiesAction = () => ({
type: CLEAR_ENTITIES,
payload: []
});
10 changes: 8 additions & 2 deletions src/store/entities.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
ENTITIES_LOADING,
CLEAR_FILTERS,
TOGGLE_TAG_MODAL,
CONFIG_CHANGED
CONFIG_CHANGED,
CLEAR_ENTITIES
} from './action-types';
import { mergeArraysByKey } from '@redhat-cloud-services/frontend-components-utilities/helpers';
import { DateFormat } from '@redhat-cloud-services/frontend-components/DateFormat';
Expand Down Expand Up @@ -123,6 +124,10 @@ function clearFilters(state) {
};
}

const clearEntities = () => {
return defaultState;
};

// eslint-disable-next-line camelcase
function entitiesLoaded(state, { payload: { results, per_page: perPage, page, count, total, loaded, filters }, meta }) {
// Older requests should not rewrite the state
Expand Down Expand Up @@ -305,5 +310,6 @@ export default {
[CLEAR_FILTERS]: clearFilters,
[ENTITIES_LOADING]: (state, { payload: { isLoading } }) => ({ ...state, loaded: !isLoading }),
[TOGGLE_TAG_MODAL]: toggleTagModalReducer,
[CONFIG_CHANGED]: (state, { payload }) => ({ ...state, invConfig: payload })
[CONFIG_CHANGED]: (state, { payload }) => ({ ...state, invConfig: payload }),
[CLEAR_ENTITIES]: clearEntities
};

0 comments on commit 97af8a3

Please sign in to comment.