Changeset View
Standalone View
binaries/data/mods/public/gui/gamesetup/Pages/MapBrowserPage/GridBrowser.js
- This file was added.
/** | |||||
* Class that arranges a grid of items using paging | |||||
* | |||||
* Needs an object as container with itemss and a object to display the page numbering (if not | |||||
elexis: ("boxed-like" -> aim for precise and complete definitions. If I see "boxed-like", I wonder what… | |||||
* make hidden object and assign it to it). | |||||
*/ | |||||
class GridBrowser | |||||
{ | |||||
constructor(container) | |||||
{ | |||||
this.container = container; | |||||
// These properties may be read from publicly | |||||
this.pageCount = undefined; | |||||
this.currentPage = undefined; | |||||
this.columnCount = undefined; | |||||
this.minColumns = undefined; | |||||
this.maxColumns = undefined; | |||||
this.rowCount = undefined; | |||||
Done Inline Actions(Perhaps its cleaner to pass undefined rather than spending 9 characters to specify an empty (aka useless) function.) elexis: (Perhaps its cleaner to pass undefined rather than spending 9 characters to specify an empty… | |||||
Done Inline ActionsEither way you have to deal with it somewhere. Having a empty functions means you don't have to bother checking for undefined. nani: Either way you have to deal with it somewhere. Having a empty functions means you don't have to… | |||||
this.itemCount = undefined; | |||||
this.itemsPerPage = undefined; | |||||
this.selected = undefined; | |||||
this.gridResizeHandlers = new Set(); | |||||
this.pageChangeHandlers = new Set(); | |||||
this.selectionChangeHandlers = new Set(); | |||||
} | |||||
registerGridResizeHandler(handler) | |||||
{ | |||||
this.gridResizeHandlers.add(handler); | |||||
Done Inline ActionsThe gridbrowser doesnt use the list array except for its length property. elexis: The gridbrowser doesnt use the `list` array except for its length property.
Therefore the… | |||||
} | |||||
registerPageChangeHandler(handler) | |||||
{ | |||||
this.pageChangeHandlers.add(handler); | |||||
} | |||||
registerSelectionChangeHandler(handler) | |||||
{ | |||||
this.selectionChangeHandlers.add(handler); | |||||
} | |||||
Done Inline ActionsclampIndex seems to add code to hide hypothetical code errors, but code errors should be reported, not hidden. elexis: clampIndex seems to add code to hide hypothetical code errors, but code errors should be… | |||||
// Inheriting classes must subscribe this event | |||||
onWindowResized() | |||||
{ | |||||
this.resizeGrid(); | |||||
this.goToPageOfSelected(); | |||||
} | |||||
setSelectedIndex(index) | |||||
{ | |||||
this.selected = index; | |||||
for (let handler of this.selectionChangeHandlers) | |||||
handler(); | |||||
} | |||||
Done Inline Actionsreturn this; never used elexis: return this; never used | |||||
Done Inline ActionsFollows the same convention as the other methods and allows for chaining if necessary. nani: Follows the same convention as the other methods and allows for chaining if necessary. | |||||
Not Done Inline ActionsAllows for chaining yes, but it was never used anywhere globally yet, using obj.statement1(); obj.statement2(); makes it more obvious that its operating on the same object (its not obvious that goToPageX returns a this instance until having read the code, especially since many functions in the class dont return this), also its not used anywhere either? If we add code, then because it really adds something, since there is a potivie cost of every line of code added (code behavior to be understood, verified and maintained). elexis: Allows for chaining yes, but it was never used anywhere globally yet, using obj.statement1()… | |||||
goToPage(pageNumber) | |||||
{ | |||||
Done Inline Actions(This clamp call also is covering up an input error, but that input error either never happens or it does happen and its hidden from the developer) elexis: (This clamp call also is covering up an input error, but that input error either never happens… | |||||
if (!Number.isInteger(pageNumber)) | |||||
throw new Error("Given argument is not a number"); | |||||
this.currentPage = pageNumber; | |||||
Done Inline ActionsWe don't have uses of template strings yet, using current + "/" + max would be more in line with string construction - but: Actually this is not just a string construction but a string used for humans to read (caption), therefore it should be localized, i.e. sprintf(translate("%(currentPage)s / %(maxPage)s"), { ... } (and the translated format string moved to the prototype) elexis: We don't have uses of template strings yet, using `current + "/" + max` would be more in line… | |||||
for (let handler of this.pageChangeHandlers) | |||||
handler(); | |||||
} | |||||
nextPage() | |||||
{ | |||||
let numberPages = Math.max(1, this.pageCount); | |||||
this.goToPage((this.currentPage + 1) % numberPages); | |||||
} | |||||
previousPage() | |||||
{ | |||||
let numberPages = Math.max(1, this.pageCount); | |||||
this.goToPage((this.currentPage + numberPages - 1) % numberPages); | |||||
} | |||||
goToPageOfSelected() | |||||
{ | |||||
this.goToPage(Math.floor(this.selected / this.itemsPerPage)); | |||||
} | |||||
Done Inline ActionsThis computation can be done when nColumns / nRows is changed. elexis: This computation can be done when nColumns / nRows is changed.
| |||||
increaseColumnCount(diff) | |||||
{ | |||||
let isSelectedInPage = | |||||
this.selected !== undefined && | |||||
Math.floor(this.selected / this.itemsPerPage) == this.currentPage; | |||||
Done Inline ActionsMath.max(1, seems unneeded (hiding an error) elexis: Math.max(1, seems unneeded (hiding an error) | |||||
this.columnCount += diff; | |||||
Done Inline ActionsgetChildOfIndex, getList function with 1 line only used in 1 line, could be inlined elexis: getChildOfIndex, getList function with 1 line only used in 1 line, could be inlined | |||||
Done Inline ActionsAlso used in MapBrowserPage.js nani: Also used in MapBrowserPage.js | |||||
this.resizeGrid(); | |||||
if (isSelectedInPage) | |||||
this.goToPageOfSelected(); | |||||
else | |||||
this.goToPage(Math.min(this.currentPage, this.pageCount - 1)); | |||||
} | |||||
resizeGrid() | |||||
{ | |||||
let size = this.container.getComputedSize(); | |||||
let width = size.right - size.left; | |||||
let height = size.bottom - size.top; | |||||
let maxColumns = Math.floor(width / this.MinItemWidth); | |||||
if (maxColumns <= 0) | |||||
return; | |||||
if (this.columnCount === undefined) | |||||
this.columnCount = Math.floor(width / this.DefaultItemWidth); | |||||
this.minColumns = Math.ceil(width / (height * this.ItemRatio)); | |||||
this.maxColumns = maxColumns; | |||||
this.columnCount = Math.min(this.maxColumns, Math.max(this.minColumns, this.columnCount)); | |||||
this.itemWidth = Math.floor(width / this.columnCount); | |||||
this.itemHeight = Math.floor(this.itemWidth / this.ItemRatio); | |||||
this.rowCount = Math.floor((size.bottom - size.top) / this.itemHeight); | |||||
this.itemsPerPage = Math.min(this.columnCount * this.rowCount, this.items.length); | |||||
this.pageCount = Math.ceil(this.itemCount / this.itemsPerPage); | |||||
Done Inline Actionsif isSelectedInPage then goToPageOfSelected is the same as goToPage(currentPage). (If I understand this if-else statement correctly, when resizing the window, the page will be selected that shows the first item of the page in the version of the page just before the resize event.) elexis: if isSelectedInPage then goToPageOfSelected is the same as goToPage(currentPage).
(If I… | |||||
for (let handler of this.gridResizeHandlers) | |||||
handler(); | |||||
} | |||||
} | |||||
Done Inline Actionst0 -> this.Margin? elexis: t0 -> this.Margin? | |||||
Done Inline Actionshardcoded for now nani: hardcoded for now | |||||
Not Done Inline ActionsI can offer to move it before committing. elexis: I can offer to move it before committing. | |||||
Done Inline Actions(no commented out code) elexis: (no commented out code) | |||||
Done Inline ActionsNot sure if it should hardcode horizontal centering. Or it it does that, use 50% size +/- offset. elexis: Not sure if it should hardcode horizontal centering. Or it it does that, use 50% size +/… |
("boxed-like" -> aim for precise and complete definitions. If I see "boxed-like", I wonder what the difference to boxed-like without quotes is. I suppose the actual restriction is image objects or GUI objects that have a sprite property)