Skip to content

Commit 7b7207c

Browse files
author
kaptron
committed
more complete refactor of loading Foamcore rows more optimistically
- also a rendering optimization for MovableGridCard, it renders a simpler GridCardPreload (gray square) before trying to render the full card
1 parent 91a56a4 commit 7b7207c

File tree

12 files changed

+245
-133
lines changed

12 files changed

+245
-133
lines changed

app/javascript/stores/RoutingStore.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ class RoutingStore extends RouterStore {
133133
this.push(path)
134134
}
135135

136+
@action
136137
beforeRouting() {
137138
const { uiStore } = this
138139
// close the org/roles menus if either are open when we route to a new page

app/javascript/stores/UiStore.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,8 @@ export default class UiStore {
100100
@observable
101101
isLoading = false
102102
@observable
103+
isRouting = false
104+
@observable
103105
isTransparentLoading = false
104106
@observable
105107
isLoadingMoveAction = false

app/javascript/stores/jsonApi/Collection.js

Lines changed: 29 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,15 @@ import SharedRecordMixin from './SharedRecordMixin'
2222
import v, { FOAMCORE_MAX_ZOOM, FOUR_WIDE_MAX_ZOOM } from '~/utils/variables'
2323
import { POPUP_ACTION_TYPES } from '~/enums/actionEnums'
2424
import { methodLibraryTags } from '~/utils/creativeDifferenceVariables'
25+
import { objectsEqual } from '~/utils/objectUtils'
2526

2627
export const ROW_ACTIONS = {
2728
INSERT: 'insert_row',
2829
REMOVE: 'remove_row',
2930
}
3031

32+
const CARD_PROPERTIES = ['id', 'updated_at', 'order', 'row', 'col']
33+
3134
class Collection extends SharedRecordMixin(BaseRecord) {
3235
static type = 'collections'
3336
static endpoint = apiUrl('collections')
@@ -621,9 +624,16 @@ class Collection extends SharedRecordMixin(BaseRecord) {
621624

622625
@computed
623626
get cardProperties() {
624-
return this.collection_cards.map(c =>
625-
_.pick(c, ['id', 'updated_at', 'order', 'row', 'col'])
626-
)
627+
return this.collection_cards.map(c => _.pick(c, CARD_PROPERTIES))
628+
}
629+
630+
@action
631+
replaceCardsIfDifferent(newCards) {
632+
const newProperties = newCards.map(c => _.pick(c, CARD_PROPERTIES))
633+
if (objectsEqual(newProperties, this.cardProperties)) {
634+
return
635+
}
636+
this.collection_cards.replace(newCards)
627637
}
628638

629639
get allowsCollectionTypeSelector() {
@@ -840,6 +850,7 @@ class Collection extends SharedRecordMixin(BaseRecord) {
840850
this.currentOrder = order
841851
}
842852
})
853+
const { uiStore } = this
843854
const params = {
844855
page,
845856
per_page,
@@ -861,6 +872,7 @@ class Collection extends SharedRecordMixin(BaseRecord) {
861872
}
862873
let apiPath
863874
if (searchTerm) {
875+
uiStore.update('isTransparentLoading', true)
864876
params.query = searchTerm
865877
if (this.collectionFilterQuery.q) {
866878
params.query += ` ${this.collectionFilterQuery.q}`
@@ -886,12 +898,14 @@ class Collection extends SharedRecordMixin(BaseRecord) {
886898
const res = await this.apiStore.request(apiPath)
887899
const { data, links, meta } = res
888900
runInAction(() => {
901+
uiStore.update('isTransparentLoading', false)
889902
if (searchTerm) {
890903
this.totalPages = (meta && meta.total_pages) || 1
891904
} else {
892905
this.totalPages = links.last
893906
}
894-
const firstPage = page === 1 && (!rows || rows[0] === 0)
907+
// NOTE: firstPage doesn't happen when loading rows
908+
const firstPage = page === 1
895909
if (
896910
firstPage &&
897911
(this.storedCacheKey !== this.cache_key ||
@@ -910,16 +924,7 @@ class Collection extends SharedRecordMixin(BaseRecord) {
910924
if (this.currentPage < page) {
911925
this.currentPage = page
912926
}
913-
const newData = _.reverse(
914-
// de-dupe merged data (deferring to new cards first)
915-
// reverse + reverse so that new cards (e.g. page 2) are replaced first but then put back at the end
916-
_.unionBy(
917-
_.reverse([...data]),
918-
_.reverse([...this.collection_cards]),
919-
'id'
920-
)
921-
)
922-
this.collection_cards.replace(newData)
927+
this.mergeCards(data)
923928
}
924929

925930
if (this.isSplitLevelBottom) {
@@ -959,9 +964,16 @@ class Collection extends SharedRecordMixin(BaseRecord) {
959964

960965
@action
961966
mergeCards = cards => {
962-
// de-dupe merged data (deferring to new cards first)
963-
const newData = _.unionBy(cards, this.collection_cards, 'id')
964-
this.collection_cards.replace(_.sortBy(newData, 'order'))
967+
const newData = _.reverse(
968+
// de-dupe merged data (deferring to new cards first)
969+
// reverse + reverse so that new cards (e.g. page 2) are replaced first but then put back at the end
970+
_.unionBy(
971+
_.reverse([...cards]),
972+
_.reverse([...this.collection_cards]),
973+
'id'
974+
)
975+
)
976+
this.collection_cards.replace(newData)
965977
}
966978

967979
API_fetchCardOrders = async () => {

app/javascript/stores/jsonApi/Item.js

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import _ from 'lodash'
2-
import { observable, runInAction } from 'mobx'
2+
import { computed, observable, runInAction } from 'mobx'
33
import { ReferenceType } from 'datx'
44

55
import { apiUrl } from '~/utils/url'
@@ -206,9 +206,19 @@ class Item extends SharedRecordMixin(BaseRecord) {
206206
})
207207
}
208208

209+
@computed
209210
get primaryDataset() {
210-
const { datasets } = this
211-
if (!datasets) return null
211+
const { apiStore } = this
212+
let { datasets } = this
213+
if (!datasets || datasets.length === 0) {
214+
// look in apiStore for any datasets that we set ds.item === this
215+
datasets = apiStore.findAll('datasets').filter(ds => {
216+
return ds.item === this
217+
})
218+
if (!datasets) {
219+
return null
220+
}
221+
}
212222
if (datasets.length === 1) return datasets[0]
213223
const primary = datasets.find(dataset => dataset.order === 0)
214224
return primary
@@ -333,7 +343,12 @@ class Item extends SharedRecordMixin(BaseRecord) {
333343
const res = await this.apiStore.request(`items/${this.id}/datasets`)
334344
runInAction(() => {
335345
this.loadingDatasets = false
336-
this.datasets = res.data
346+
const datasets = res.data
347+
_.each(datasets, d => {
348+
// set the dataset item reference in case we need to look it up later
349+
d.item = this
350+
})
351+
this.datasets = datasets
337352
})
338353
return this.datasets
339354
}

app/javascript/ui/Routes.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,15 @@ class Routes extends React.Component {
116116
})
117117
}
118118

119+
componentDidUpdate(prevProps) {
120+
const { uiStore } = this.props
121+
if (this.props.location !== prevProps.location) {
122+
uiStore.update('isRouting', true)
123+
} else if (uiStore.isRouting) {
124+
uiStore.update('isRouting', false)
125+
}
126+
}
127+
119128
componentWillUnmount() {
120129
document.removeEventListener('keydown', captureGlobalKeypress)
121130
document.removeEventListener('touchmove', this.handleTouchMove, {

app/javascript/ui/grid/FoamcoreGrid.js

Lines changed: 43 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ class FoamcoreGrid extends React.Component {
9090
const { collection, uiStore } = this.props
9191
if (collection.id !== prevProps.collection.id) {
9292
uiStore.determineZoomLevels(collection)
93+
this.loadInitialRows()
9394
}
9495

9596
if (!objectsEqual(this.props.cardProperties, prevProps.cardProperties)) {
@@ -141,9 +142,8 @@ class FoamcoreGrid extends React.Component {
141142
this.handleZoomIn()
142143
}
143144

144-
// loadMoreRows via scrolling is just for infinite scroll / splitLevelBottom
145145
const visRows = uiStore.visibleRows
146-
if (!collection.isSplitLevelBottom || !visRows) {
146+
if (!visRows) {
147147
return
148148
}
149149

@@ -155,31 +155,44 @@ class FoamcoreGrid extends React.Component {
155155
}
156156
}
157157

158-
loadInitialRows = () => {
158+
loadInitialRows = async () => {
159159
const { collection, loadCollectionCards } = this.props
160-
const { loadedRows } = collection
161-
// const { loadMoreCollectionCards } = this
160+
if (collection.isSplitLevelBottom) {
161+
// this is only used for "normal" grids
162+
return
163+
}
164+
162165
// arbitrary 300 row initial limit?
163166
const maxRow = _.min([collection.max_row_index, 300])
164167

165168
const rowsPerPage = 30
166-
let min = loadedRows
167-
let max = loadedRows + rowsPerPage
168-
console.log({ maxRow })
169+
let min = 0
170+
let max = rowsPerPage
171+
const requests = []
169172
while (min <= maxRow) {
170173
// just fire off multiple async requests (without awaiting)
171-
console.log('loadCollectionCards', [min, max])
172-
loadCollectionCards({
173-
// just load by row # downward, and always load all 16 cols
174-
rows: [min, max],
174+
const promise = new Promise(resolve => {
175+
const rows = [min, max]
176+
return resolve(
177+
loadCollectionCards({
178+
// just load by row # downward, and always load all 16 cols
179+
rows,
180+
})
181+
)
175182
})
183+
requests.push(promise)
176184
min = max + 1
177185
max = max + rowsPerPage
178186
}
187+
const data = await Promise.all(requests)
188+
runInAction(() => {
189+
const newCards = _.flatten(data)
190+
collection.replaceCardsIfDifferent(newCards)
191+
})
179192
}
180193

181194
loadMoreRows = () => {
182-
const { collection } = this.props
195+
const { collection, uiStore } = this.props
183196
const { loadMoreCollectionCards } = this
184197
if (collection.isSplitLevelBottom) {
185198
if (collection.hasMore) {
@@ -188,22 +201,23 @@ class FoamcoreGrid extends React.Component {
188201
return
189202
}
190203

191-
// const visRows = uiStore.visibleRows
192-
// const collectionMaxRow = collection.max_row_index
193-
// // min row should start with the next row after what's loaded
194-
// const loadMinRow = collection.loadedRows + 1
195-
// // add a buffer of 3 more rows (constrained by max row on collection)
196-
// const loadMaxRow = _.min([
197-
// collectionMaxRow,
198-
// Math.ceil(loadMinRow + visRows.num + 3),
199-
// ])
200-
// // min and max could be equal if there is one more row to load
201-
// if (loadMinRow <= loadMaxRow) {
202-
// return loadMoreCollectionCards({
203-
// // just load by row # downward, and always load all 16 cols
204-
// rows: [loadMinRow, loadMaxRow],
205-
// })
206-
// }
204+
// NOTE: you should really only get into here if you go past row 300+
205+
const visRows = uiStore.visibleRows
206+
const collectionMaxRow = collection.max_row_index
207+
// min row should start with the next row after what's loaded
208+
const loadMinRow = collection.loadedRows + 1
209+
// add a buffer of 3 more rows (constrained by max row on collection)
210+
const loadMaxRow = _.min([
211+
collectionMaxRow,
212+
Math.ceil(loadMinRow + visRows.num + 3),
213+
])
214+
// min and max could be equal if there is one more row to load
215+
if (loadMinRow <= loadMaxRow) {
216+
return loadMoreCollectionCards({
217+
// just load by row # downward, and always load all 16 cols
218+
rows: [loadMinRow, loadMaxRow],
219+
})
220+
}
207221
}
208222

209223
loadMoreCollectionCards = async (opts = {}) => {

app/javascript/ui/grid/MovableGridCard.js

Lines changed: 33 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,12 @@ import ResizeIcon from '~/ui/icons/ResizeIcon'
1919
import { StyledCardWrapper } from '~/ui/grid/shared'
2020
import { pageBoundsScroller } from '~/utils/ScrollNearPageBoundsService'
2121

22+
const GridCardPreload = styled.div`
23+
height: 100%;
24+
width: 100%;
25+
background: ${v.colors.commonMedium};
26+
`
27+
2228
const StyledResizeIcon = styled.div`
2329
position: absolute;
2430
/* hide the resize icon while the menu is open so they don't overlap */
@@ -88,13 +94,26 @@ class MovableGridCard extends React.Component {
8894
resizeWidth: 0,
8995
resizeHeight: 0,
9096
allowTouchDeviceDragging: false,
97+
// if we're mid-routing change, then set to preloading gray square
98+
preloading: uiStore.isRouting,
9199
}
92100
this.debouncedAllowTouchDeviceDrag = _.debounce(() => {
93101
if (this.unmounted) return
94102
this.setState({ allowTouchDeviceDragging: true })
95103
}, v.touchDeviceHoldToDragTime)
96104
}
97105

106+
componentDidMount() {
107+
if (!this.state.preloading) {
108+
return
109+
}
110+
setTimeout(() => {
111+
if (this.unmounted) return
112+
// after a slight delay, turn preloading off and render the actual GridCard
113+
this.setState({ preloading: false })
114+
}, 150)
115+
}
116+
98117
componentDidUpdate(prevProps) {
99118
if (this.state.dragging || this.unmounted) {
100119
return
@@ -457,6 +476,7 @@ class MovableGridCard extends React.Component {
457476
resizeHeight,
458477
x,
459478
y,
479+
preloading,
460480
} = this.state
461481

462482
const {
@@ -564,7 +584,9 @@ class MovableGridCard extends React.Component {
564584
const adjustedWidth = (width + resizeWidth) / zoomLevel
565585
const adjustedHeight = (height + resizeHeight) / zoomLevel
566586
let transition =
567-
dragging || resizing || currentlyZooming ? 'none' : cardCSSTransition
587+
dragging || resizing || currentlyZooming || preloading
588+
? 'none'
589+
: cardCSSTransition
568590
// TODO this should actually check it's a breadcrumb
569591
const draggedOverBreadcrumb = !!activeDragTarget
570592
if (dragging && this.state.allowTouchDeviceDragging) {
@@ -705,12 +727,16 @@ class MovableGridCard extends React.Component {
705727
transform={transform}
706728
zoomLevel={zoomLevel}
707729
>
708-
<GridCard
709-
{...cardProps}
710-
draggingMultiple={draggingMultiple}
711-
hoveringOver={hoveringOverRight}
712-
zoomLevel={zoomLevel}
713-
/>
730+
{/* During preload we just render a gray square to simplify initial render */}
731+
{preloading && <GridCardPreload />}
732+
{!preloading && (
733+
<GridCard
734+
{...cardProps}
735+
draggingMultiple={draggingMultiple}
736+
hoveringOver={hoveringOverRight}
737+
zoomLevel={zoomLevel}
738+
/>
739+
)}
714740
</InnerCardWrapper>
715741
</Rnd>
716742
</StyledCardWrapper>

app/javascript/ui/grid/covers/DataItemCover.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,10 +30,10 @@ class DataItemCover extends React.Component {
3030
}
3131
}
3232

33-
async loadDatasets() {
33+
loadDatasets() {
3434
const { item } = this.props
3535
if (!item.loadingDatasets) {
36-
await item.API_fetchDatasets()
36+
item.API_fetchDatasets()
3737
}
3838
}
3939

0 commit comments

Comments
 (0)