refactor(entity-list): add num of rows preference feature #3327
Conversation
Refs: TOCDEV-5494 Changelog: add feature to save number of rows as user preference in entity list
| return { | ||
| sortable: state.list.sortable, | ||
| disablePreferencesMenu: state.list.disablePreferencesMenu, | ||
| numOfRows: state.preferences.numOfRows |
There was a problem hiding this comment.
nipick: numOfRows is not needed on the NavigationCellHeader and could be removed
| @@ -33,6 +35,7 @@ export function* loadPreferences() { | |||
| yield put(setPositions(util.getPositions(preferences))) | |||
| yield put(setSorting(util.getSorting(preferences))) | |||
| yield put(setColumns(util.getColumns(preferences))) | |||
| yield put(actions.setNumberOfTableRows(Number(preferences[`${formName}.numOfRows`]))) | |||
There was a problem hiding this comment.
suggestion(non-blocking): create util.getNumberOfTableRows in util similar as the others
For all the other preferences we have util functions to get those preferences.
We could move this logic also to the util and have some tests for proper handling of the type conversion.
| const {numOfRows} = yield take(answerChannel) | ||
| const inputState = yield select(inputSelector) | ||
|
|
||
| yield put(setNumberOfTableRows(Number(numOfRows))) |
There was a problem hiding this comment.
suggestion(non-blocking): how about moving the type conversion one step higher
Here we need to apply the type conversion twice: Line 86 and Line 88
How about make the type conversion inside the displayTableRowsModal function in the onOk callback on Line 148.
Then the conversion has only to be done once and it would be a bit easier to maintain.
Or it could also already be done int he SelectNumRows component.
There was a problem hiding this comment.
These conversions can be removed, when we use the key as value instead the display
| @@ -299,7 +299,7 @@ export function* fetchEntitiesAndAddToStore(page) { | |||
| ...basicQuery, | |||
| page, | |||
| sorting, | |||
| limit, | |||
| limit: numOfRows || limit, | |||
There was a problem hiding this comment.
suggestion(non-blocking): move to helper function
We need to apply this "fallback" logic in 3 places now. We could have a util function
const selectActualLimit = (state) => state.preferences.numOfRows || state.input.limit
This could be used in all three places:
Line 28 (File: TableContainer) & Line 302: limit: selectActualLimit(state)
Line 344: const actualLimit = yield select(selectActualLimit)
| </StyledEditableValueWrapper> | ||
| <StyledButtonWrapper> | ||
| <Button onClick={() => onOk(value?.display)} look={'raised'}> | ||
| OK |
There was a problem hiding this comment.
todo: we should use our text resources here as well
| events={{onChange: setValue}} | ||
| options={{ | ||
| options, | ||
| noResultsText: 'no results found', |
There was a problem hiding this comment.
todo: we should use out text resources here as well
|
|
||
| const SelectNumRows = ({onOk, numOfRows}) => { | ||
| const options = [ | ||
| {key: 1, display: '25'}, |
There was a problem hiding this comment.
suggestion(blocking): Use number value as key and use key for your value instead display
const options = [{key: 25: display: '25'}, ...]
const matchingValue = options.find(o => o.key === numOfRows)
...
onClick={() => onOk(value?.key)}
Then we could maybe remove all the type conversions expect the one when loading the preferences initially.
Refs: TOCDEV-5494
yilkur
force-pushed
the
pr/num-of-rows-preference
branch
from
0bdae3b to
6e4bdce
Compare
Refs: TOCDEV-5494
Changelog: add feature to save number of rows as user preference in entity list