Adds multi-select support#1360
Conversation
| let imageHistoryBrowser = new GenPageBrowserClass('image_history', listOutputHistoryFolderAndFiles, 'imagehistorybrowser', 'Thumbnails', describeOutputFile, selectOutputInHistory, | ||
| `<label for="image_history_sort_by">Sort:</label> <select id="image_history_sort_by"><option>Name</option><option>Date</option></select> <input type="checkbox" id="image_history_sort_reverse"> <label for="image_history_sort_reverse">Reverse</label>   <input type="checkbox" id="image_history_allow_anims" checked autocomplete="off"> <label for="image_history_allow_anims">Allow Animation</label>`); | ||
| imageHistoryBrowser.enableBrowserMultiSelect = true; | ||
| imageHistoryBrowser.keepBrowserMultiSelectKeyAfterPrune = function(key, namesInCurrentList) { |
There was a problem hiding this comment.
match existing patterns for function declarations
|
|
||
| function clickImageInBatch(div) { | ||
| let imgElem = div.getElementsByTagName('img')[0]; | ||
| let multiSelectKey = getImageFullSrc(div.dataset.src); |
There was a problem hiding this comment.
there shouldn't be any multiselect handling outside of the browser class like this
| } | ||
| let isStarred = this.isStarred(model.data.name); | ||
| let starButton = { label: isStarred ? 'Unstar' : 'Star', onclick: () => { this.toggleStar(model.data.name); } }; | ||
| let starButton = { label: isStarred ? 'Unstar' : 'Star', onclick: () => { this.toggleStar(model.data.name); }, can_multi: true }; |
There was a problem hiding this comment.
Are you objecting to the , can_multi: true flag itself (how I've defined it, for example), or objecting to Star/Unstar action being available for multi-select action?
There was a problem hiding this comment.
you editing the way star is handled for multi, which is already covered properly for images, and presets/models should handle it the same as images
There was a problem hiding this comment.
Probably the best move is limit the PR to browser itself & images, preset/models can be separate
aka the universal rule of PR contributing: minimize the scope of edits per individual PR
| toggleStar(fullsrc, src); | ||
| } | ||
| }, | ||
| can_multi: true |
| if (div) { | ||
| removeImageBlockFromBatch(div); | ||
| } | ||
| if (imageHistoryBrowser.enableBrowserMultiSelect) { |
There was a problem hiding this comment.
this is questionably placed aaand also seems to be a mixup between the 'enable' and 'active' bools?
| return true; | ||
| } | ||
| let currentImageBatchDiv = getRequiredElementById('current_image_batch'); | ||
| for (let candidate of currentImageBatchDiv.querySelectorAll('.image-block:not(.image-block-placeholder)')) { |
| { label: 'Direct Apply', onclick: () => applyOnePreset(preset.data) }, | ||
| { label: preset.data.is_starred ? 'Unstar' : 'Star', onclick: () => togglePresetStar(preset) }, | ||
| { label: 'Direct Apply', onclick: () => applyOnePreset(preset.data), can_multi: true }, | ||
| { label: preset.data.is_starred ? 'Unstar' : 'Star', onclick: () => togglePresetStar(preset), can_multi: true }, |
| { label: 'Edit Preset', onclick: () => editPreset(preset.data) }, | ||
| { label: 'Duplicate Preset', onclick: () => duplicatePreset(preset.data) }, | ||
| { label: 'Export Preset', onclick: () => exportOnePresetButton(preset.data) }, | ||
| { label: 'Export Preset', onclick: () => exportOnePresetButton(preset.data), can_multi: true }, |
There was a problem hiding this comment.
this can't function natively and needs separate impl
| this.runAfterUpdate = []; | ||
| this.refreshHandler = (callback) => callback(); | ||
| this.checkIsSmall(); | ||
| this.enableBrowserMultiSelect = false; |
There was a problem hiding this comment.
should probably be allowMultiSelect
| this.checkIsSmall(); | ||
| this.enableBrowserMultiSelect = false; | ||
| this.multiSelectActive = false; | ||
| this.multiSelectedKeys = new Set(); |
There was a problem hiding this comment.
this is liable to lead to state tracking inconsistencies, as jank as it feels putting class or .dataset's on the block divs is probably the stablest way to track
| textBlock.tabIndex = 0; | ||
| textBlock.innerHTML = desc.description; | ||
| div.appendChild(textBlock); | ||
| div.addEventListener('click', (e) => { |
There was a problem hiding this comment.
don't need a separate click handler, the select handler should cover it
2 participants
CleanShot.2026-04-29.at.23.31.21.mp4