Skip to content
This repository was archived by the owner on Dec 28, 2022. It is now read-only.

refactor(entity-list): add num of rows preference feature #3327

Open
yilkur wants to merge 2 commits into
base: master
Choose a base branch
from
Open

refactor(entity-list): add num of rows preference feature #3327

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f10ea12
refactor(entity-list): add num of rows preference feature
yilkur Dec 23, 2022
6e4bdce
chore: fix review issues
yilkur Dec 23, 2022
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
5 changes: 2 additions & 3 deletions packages/core/app-extensions/src/notification/modules/modal/ModalDisplay/StyledComponents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import styled, {createGlobalStyle} from 'styled-components'
import {Button, scale, StyledScrollbar, StyledTether, theme} from 'tocco-ui'
import {Button, scale, StyledScrollbar, StyledTether, themeSelector} from 'tocco-ui'

export const basePadding = scale.space(0.5)

export const StyledModalContent = styled.div`
position: relative;
background-color: ${theme.color('paper')};
background-color: ${themeSelector.color('paper')};
padding: ${basePadding};
display: grid;
grid-template-rows: [header] auto [body] 1fr;
Expand Down Expand Up @@ -77,7 +77,6 @@ export const StyledTitleWrapper = styled.div`
export const StyledModalBody = styled.div`
grid-row-start: body;
overflow: hidden auto;
padding-right: ${scale.space(0)};
${StyledScrollbar}
`

Expand Down
45 changes: 21 additions & 24 deletions packages/core/entity-list/src/components/Table/NavigationCellHeader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,33 @@
import PropTypes from 'prop-types'
import {FormattedMessage} from 'react-intl'
import {connect} from 'react-redux'
import {BallMenu, MenuItem} from 'tocco-ui'

import {displayColumnModal, resetSorting, resetPreferences, resetColumns} from '../../modules/preferences/actions'

const NavigationCellHeader = props =>
!props.disablePreferencesMenu ? (
const NavigationCellHeader = ({
disablePreferencesMenu,
displayColumnModal,
resetColumns,
sortable,
resetSorting,
resetPreferences,
displayTableRowsModal
}) =>
!disablePreferencesMenu ? (
<BallMenu buttonProps={{icon: 'ellipsis-v'}}>
<MenuItem onClick={props.displayColumnModal}>
<MenuItem onClick={displayColumnModal}>
<FormattedMessage id="client.entity-list.preferences.columns" />
</MenuItem>
<MenuItem onClick={props.resetColumns}>
<MenuItem onClick={displayTableRowsModal}>
<FormattedMessage id="client.entity-list.preferences.numOfRows" />
</MenuItem>
<MenuItem onClick={resetColumns}>
<FormattedMessage id="client.entity-list.preferences.columns.reset" />
</MenuItem>
{props.sortable && (
<MenuItem onClick={props.resetSorting}>
{sortable && (
<MenuItem onClick={resetSorting}>
<FormattedMessage id="client.entity-list.sorting.reset" />
</MenuItem>
)}
<MenuItem onClick={props.resetPreferences}>
<MenuItem onClick={resetPreferences}>
<FormattedMessage id="client.entity-list.preferences.reset" />
</MenuItem>
</BallMenu>
Expand All @@ -31,19 +39,8 @@ NavigationCellHeader.propTypes = {
resetPreferences: PropTypes.func.isRequired,
resetColumns: PropTypes.func.isRequired,
sortable: PropTypes.bool,
disablePreferencesMenu: PropTypes.bool
disablePreferencesMenu: PropTypes.bool,
displayTableRowsModal: PropTypes.func.isRequired
}

const mapActionCreators = {
displayColumnModal,
resetSorting,
resetPreferences,
resetColumns
}

const mapStateToProps = (state, props) => ({
sortable: state.input.sortable,
disablePreferencesMenu: state.list.disablePreferencesMenu
})

export default connect(mapStateToProps, mapActionCreators)(NavigationCellHeader)
export default NavigationCellHeader
28 changes: 28 additions & 0 deletions packages/core/entity-list/src/components/Table/NavigationCellHeaderContainer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import {connect} from 'react-redux'

import {
displayColumnModal,
resetSorting,
resetPreferences,
resetColumns,
displayTableRowsModal
} from '../../modules/preferences/actions'
import NavigationCellHeader from './NavigationCellHeader'

const mapActionCreators = {
displayColumnModal,
resetSorting,
resetPreferences,
resetColumns,
displayTableRowsModal
}

const mapStateToProps = (state, props) => {
return {
sortable: state.list.sortable,
disablePreferencesMenu: state.list.disablePreferencesMenu,
numOfRows: state.preferences.numOfRows
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

nipick: numOfRows is not needed on the NavigationCellHeader and could be removed

}
}

export default connect(mapStateToProps, mapActionCreators)(NavigationCellHeader)
44 changes: 44 additions & 0 deletions packages/core/entity-list/src/components/Table/SelectRowNums.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import PropTypes from 'prop-types'
import {useState} from 'react'
import {EditableValue, Button} from 'tocco-ui'

import {StyledButtonWrapper, StyledEditableValueWrapper} from './StyledComponents'

const SelectNumRows = ({onOk, numOfRows}) => {
const options = [
{key: 1, display: '25'},
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

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.

{key: 2, display: '50'},
{key: 3, display: '100'}
]
const matchingValue = options.find(option => Number(option.display) === numOfRows)
const [value, setValue] = useState(matchingValue)

return (
<>
<StyledEditableValueWrapper>
<EditableValue
value={value}
type="single-select"
events={{onChange: setValue}}
options={{
options,
noResultsText: 'no results found',
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

todo: we should use out text resources here as well

isLoading: false
}}
/>
</StyledEditableValueWrapper>
<StyledButtonWrapper>
<Button onClick={() => onOk(value?.display)} look={'raised'}>
OK
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

todo: we should use our text resources here as well

</Button>
</StyledButtonWrapper>
</>
)
}

SelectNumRows.propTypes = {
onOk: PropTypes.func.isRequired,
numOfRows: PropTypes.number
}

export default SelectNumRows
21 changes: 20 additions & 1 deletion packages/core/entity-list/src/components/Table/StyledComponents.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import styled from 'styled-components'
import {themeSelector, declareFont} from 'tocco-ui'
import {themeSelector, declareFont, scale, StyledButton, colorizeBorder} from 'tocco-ui'

export const StyledMarkingWrapper = styled.span`
${declareFont()}
Expand All @@ -13,3 +13,22 @@ export const StyledMarkingWrapper = styled.span`
}
${({marked, theme}) => marked && `color: ${theme.colors.secondary};`}
`

export const StyledButtonWrapper = styled.div`
position: sticky;
bottom: 0;
padding-top: ${scale.space(0)};
background-color: ${themeSelector.color('paper')};
display: flex;
justify-content: flex-end;

${StyledButton} {
margin-right: 0;
}
`

export const StyledEditableValueWrapper = styled.div`
border: 1px solid ${colorizeBorder.shade1};
padding-right: ${scale.space(-1)};
padding-left: ${scale.space(-1)};
`
2 changes: 1 addition & 1 deletion packages/core/entity-list/src/components/Table/navigationCell.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import PropTypes from 'prop-types'
import {Icon} from 'tocco-ui'

import NavigationCellHeader from './NavigationCellHeader'
import NavigationCellHeader from './NavigationCellHeaderContainer'

const CellRenderer = ({showNavigation, rowData, navigationStrategy, parent}) =>
showNavigation && navigationStrategy.DetailLinkRelative ? (
Expand Down
2 changes: 1 addition & 1 deletion packages/core/entity-list/src/containers/TableContainer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const mapStateToProps = (state, props) => ({
currentPage: state.list.currentPage,
entities: state.list.entities,
entityCount: state.list.entityCount,
limit: state.input.limit,
limit: state.preferences.numOfRows || state.input.limit,
inProgress: state.list.inProgress,
tableSelectionStyle: getTableSelectionStyle(state.input.selectionStyle, getSelectable(getFormDefinition(state))),
clickable: getClickable(getFormDefinition(state)),
Expand Down
8 changes: 5 additions & 3 deletions packages/core/entity-list/src/modules/list/sagas.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ export function* setLazyDataMarked(entityName, markings) {
export function* fetchEntitiesAndAddToStore(page) {
const state = yield select(stateSelector)
const {entityName, scope, limit} = state.input
const {columns: columnPreferences} = state.preferences
const {columns: columnPreferences, numOfRows} = state.preferences
const {entityStore, sorting} = state.list
if (!entityStore[page]) {
const basicQuery = yield call(getBasicQuery)
Expand All @@ -299,7 +299,7 @@ export function* fetchEntitiesAndAddToStore(page) {
...basicQuery,
page,
sorting,
limit,
limit: numOfRows || limit,
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

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)

paths
}

Expand Down Expand Up @@ -340,6 +340,8 @@ export function* delayedPreloadNextPage(page) {

export function* preloadNextPage(currentPage) {
const {limit} = yield select(inputSelector)
const {numOfRows} = yield select(preferencesSelector)
const actualLimit = numOfRows || limit
const list = yield select(listSelector)
const {entityStore} = list
let {entityCount} = list
Expand All @@ -350,7 +352,7 @@ export function* preloadNextPage(currentPage) {
entityCount = setCountAction.payload.entityCount
}

if (currentPage * limit < entityCount && !entityStore[nextPage]) {
if (currentPage * actualLimit < entityCount && !entityStore[nextPage]) {
yield call(fetchEntitiesAndAddToStore, nextPage)
}
}
Expand Down
11 changes: 11 additions & 0 deletions packages/core/entity-list/src/modules/preferences/actions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ export const RESET_COLUMNS = 'preferences/RESET_COLUMNS'
export const RESET_PREFERENCES = 'preferences/RESET_PREFERENCES'
export const DISPLAY_COLUMN_MODAL = 'preferences/DISPLAY_COLUMN_MODAL'
export const SET_PREFERENCES_LOADED = 'preferences/SET_PREFERENCES_LOADED'
export const SET_NUMBER_OF_TABLE_ROWS = 'preferences/SET_NUMBER_OF_TABLE_ROWS'
export const DISPLAY_TABLE_ROWS_MODAL = 'preferences/DISPLAY_TABLE_ROWS_MODAL'

export const loadPreferences = () => ({
type: LOAD_PREFERENCES
Expand Down Expand Up @@ -78,3 +80,12 @@ export const resetColumns = () => ({
type: RESET_COLUMNS,
payload: {}
})

export const displayTableRowsModal = () => ({
type: DISPLAY_TABLE_ROWS_MODAL
})

export const setNumberOfTableRows = numOfRows => ({
type: SET_NUMBER_OF_TABLE_ROWS,
payload: {numOfRows}
})
12 changes: 10 additions & 2 deletions packages/core/entity-list/src/modules/preferences/reducer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,13 @@ const resetPreferences = state => ({
...state,
positions: {},
sorting: [],
columns: {}
columns: {},
numOfRows: undefined
})

const setNumberOfTableRows = (state, {payload: {numOfRows}}) => ({
...state,
numOfRows
})

const ACTION_HANDLERS = {
Expand All @@ -36,14 +42,16 @@ const ACTION_HANDLERS = {
[actions.SET_PREFERENCES_LOADED]: reducerUtil.singleTransferReducer('preferencesLoaded'),
[actions.RESET_SORTING]: resetSorting,
[actions.RESET_COLUMNS]: resetColumns,
[actions.RESET_PREFERENCES]: resetPreferences
[actions.RESET_PREFERENCES]: resetPreferences,
[actions.SET_NUMBER_OF_TABLE_ROWS]: setNumberOfTableRows
}

const initialState = {
positions: {},
sorting: [],
columns: {},
widths: {},
numOfRows: undefined,
preferencesLoaded: false
}

Expand Down
41 changes: 39 additions & 2 deletions packages/core/entity-list/src/modules/preferences/sagas.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@ import {all, call, put, select, take, takeLatest} from 'redux-saga/effects'
import {rest, notification} from 'tocco-app-extensions'

import ColumnModal from '../../components/ColumnModal'
import SelectNumRows from '../../components/Table/SelectRowNums'
import {getTableColumns} from '../../util/api/forms'
import * as util from '../../util/preferences'
import * as listActions from '../list/actions'
import * as listSagas from '../list/sagas'
import {setPositions, setSorting, setColumns, setPreferencesLoaded} from './actions'
import {setPositions, setSorting, setColumns, setPreferencesLoaded, setNumberOfTableRows} from './actions'
import * as actions from './actions'

export const inputSelector = state => state.input
Expand All @@ -22,7 +23,8 @@ export default function* sagas() {
takeLatest(actions.RESET_SORTING, resetSorting),
takeLatest(actions.RESET_COLUMNS, resetColumns),
takeLatest(actions.RESET_PREFERENCES, resetPreferences),
takeLatest(actions.DISPLAY_COLUMN_MODAL, displayColumnModal)
takeLatest(actions.DISPLAY_COLUMN_MODAL, displayColumnModal),
takeLatest(actions.DISPLAY_TABLE_ROWS_MODAL, displayTableRowsModal)
])
}

Expand All @@ -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`])))
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

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.

yield put(setPreferencesLoaded(true))
}

Expand Down Expand Up @@ -76,6 +79,15 @@ export function* saveSorting() {
}
}

export function* saveNumberOfTableRows(answerChannel) {
const {numOfRows} = yield take(answerChannel)
const inputState = yield select(inputSelector)

yield put(setNumberOfTableRows(Number(numOfRows)))
Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Contributor

@izzappel izzappel Dec 23, 2022

Choose a reason for hiding this comment

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

These conversions can be removed, when we use the key as value instead the display

yield call(listSagas.reloadData)
yield call(rest.savePreferences, {[`${inputState.formName}_${inputState.scope}.numOfRows`]: Number(numOfRows)})
}

export function* resetSorting() {
const inputState = yield select(inputSelector)
yield all([
Expand Down Expand Up @@ -120,6 +132,31 @@ export function* displayColumnModal() {
yield saveColumnPreferences(answerChannel, preferencesColumns, formDefinition)
}

export function* displayTableRowsModal() {
const {formDefinition} = yield select(listSagas.listSelector)
const answerChannel = yield call(channel)
const {numOfRows: preferencesNumOfRows} = yield select(preferencesSelector)

yield put(
notification.modal(
`${formDefinition.id}-numOfRows-setting`,
'client.entity-list.preferences.numOfRows',
null,
({close}) => {
const onOk = numOfRows => {
close()
answerChannel.put({numOfRows})
}

return <SelectNumRows onOk={onOk} numOfRows={preferencesNumOfRows} />
},
true
)
)

yield call(saveNumberOfTableRows, answerChannel)
}

function* saveColumnPreferences(answerChannel, preferencesColumns, formDefinition) {
const columns = (yield take(answerChannel)).reduce(
(accumulator, item) => ({
Expand Down