-
Notifications
You must be signed in to change notification settings - Fork 4
Extensions #385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Extensions #385
Changes from all commits
20f67be
40b1b00
518e3f0
7a60c4e
4959f15
48819ed
d6f5093
70854c1
bc460ce
81c5747
6d3a80d
2da035d
86d51a8
91b6fc5
a2ab8fe
b3b7823
1581487
6d1582a
0af3d7d
ed4ad28
b57b624
a831eae
e231fa5
544df95
b014b27
6581a94
187779f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import React from 'react'; | ||
| import PropTypes from 'prop-types'; | ||
| import {FormattedMessage} from 'react-intl'; | ||
| import {APP_NAME} from '../../lib/brand'; | ||
|
|
||
| const UnsandboxModal = props => ( | ||
| <div> | ||
| <p> | ||
| <FormattedMessage | ||
| defaultMessage="The extension {extensionName} wants to run without the sandbox." | ||
| description="Part of modal shown when an extension asks to run unsandboxed" | ||
| id="tw.unsandbox.title" | ||
| values={{ | ||
| extensionName: props.extensionName | ||
| }} | ||
| /> | ||
| </p> | ||
| <p> | ||
| <FormattedMessage | ||
| // eslint-disable-next-line max-len | ||
| defaultMessage="Running without the sandbox is dangerous. It can delete your projects, steal credentials, install malware, or perform other harmful actions. Only allow this if you trust the extension author. The {APP_NAME} developers are not responsible for any resulting issues." | ||
| description="Part of modal shown when an extension asks to run unsandboxed" | ||
| // garbomuffin level of fearmongering lol | ||
| // ok sorry garbomuffin it's just that literally all the modals made by garbomuffin are so exaggeratedly dramatic that I can't help but poke fun at it | ||
|
Check failure on line 24 in src/components/tw-security-manager-modal/unsandbox.jsx
|
||
| id="tw.unsandbox.warning" | ||
| values={{ | ||
| APP_NAME | ||
| }} | ||
| /> | ||
| </p> | ||
| </div> | ||
| ); | ||
|
|
||
| UnsandboxModal.propTypes = { | ||
| extensionName: PropTypes.string.isRequired | ||
| }; | ||
|
|
||
| export default UnsandboxModal; | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -144,6 +144,23 @@ class Blocks extends React.Component { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.ScratchBlocks.Procedures.externalProcedureDefCallback = this.props.onActivateCustomProcedures; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.ScratchBlocks.ScratchMsgs.setLocale(this.props.locale); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Bridge FieldCustom from Closure-compiled Blockly → ScratchBlocks for the JS extension. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // The develop-builds bundle defines window.Blockly.FieldCustom, while extensions call | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ScratchBlocks.FieldCustom.registerInput(...). This assignment wires them together. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+147
to
+149
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (window.Blockly && window.Blockly.FieldCustom) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.ScratchBlocks.FieldCustom = window.Blockly.FieldCustom; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Temporary guard: keep UI stable even if bundles momentarily lack FieldCustom. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (this.ScratchBlocks && !this.ScratchBlocks.FieldCustom) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.ScratchBlocks.FieldCustom = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| registerInput: () => {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| unregisterInput: () => {}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| getRegisteredInputs: () => new Map() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
ampelectrecuted marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| log.warn('ScratchBlocks.FieldCustom is not available; using stub implementation. ' + | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'This usually means the Blockly → ScratchBlocks FieldCustom bridge is not configured correctly.'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+155
to
+161
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.ScratchBlocks.FieldCustom = { | |
| registerInput: () => {}, | |
| unregisterInput: () => {}, | |
| getRegisteredInputs: () => new Map() | |
| }; | |
| log.warn('ScratchBlocks.FieldCustom is not available; using stub implementation. ' + | |
| 'This usually means the Blockly → ScratchBlocks FieldCustom bridge is not configured correctly.'); | |
| const fieldCustomStub = { | |
| registerInput: (...args) => { | |
| log.error( | |
| 'ScratchBlocks.FieldCustom.registerInput was called, but ScratchBlocks.FieldCustom ' + | |
| 'is using a stub implementation because the Blockly → ScratchBlocks FieldCustom bridge ' + | |
| 'is not configured. This usually means the develop-builds bundle that defines ' + | |
| 'window.Blockly.FieldCustom was not loaded. JS extensions that rely on custom fields ' + | |
| 'will not work until this is fixed.', | |
| {args} | |
| ); | |
| }, | |
| unregisterInput: (...args) => { | |
| log.error( | |
| 'ScratchBlocks.FieldCustom.unregisterInput was called, but ScratchBlocks.FieldCustom ' + | |
| 'is using a stub implementation because the Blockly → ScratchBlocks FieldCustom bridge ' + | |
| 'is not configured. This usually means the develop-builds bundle that defines ' + | |
| 'window.Blockly.FieldCustom was not loaded.', | |
| {args} | |
| ); | |
| }, | |
| getRegisteredInputs: () => { | |
| log.error( | |
| 'ScratchBlocks.FieldCustom.getRegisteredInputs was called, but ScratchBlocks.FieldCustom ' + | |
| 'is using a stub implementation because the Blockly → ScratchBlocks FieldCustom bridge ' + | |
| 'is not configured. Returning an empty Map; JS extensions that rely on custom fields ' + | |
| 'will not behave correctly until the Blockly bundle exposing window.Blockly.FieldCustom ' + | |
| 'is loaded.' | |
| ); | |
| return new Map(); | |
| } | |
| }; | |
| this.ScratchBlocks.FieldCustom = fieldCustomStub; | |
| log.warn( | |
| 'ScratchBlocks.FieldCustom is not available; using stub implementation. ' + | |
| 'This typically means window.Blockly.FieldCustom was not provided by the develop-builds ' + | |
| 'bundle. JS extensions that register custom fields will not function. Make sure the Blockly ' + | |
| 'bundle that exposes window.Blockly.FieldCustom is loaded before mounting the <Blocks /> component.' | |
| ); |
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fallback FieldCustom implementation has no-op methods that return undefined or an empty Map. Consider adding console warnings in these no-op methods to help developers identify when they're working with the fallback implementation rather than the real one. This would aid debugging when window.Blockly.FieldCustom is missing.
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conditional logic on line 154 creates a stub implementation only when ScratchBlocks exists but FieldCustom doesn't. However, if ScratchBlocks itself doesn't exist, this code won't run at all, which could lead to runtime errors later. Consider adding a check for when ScratchBlocks is undefined: if (!this.ScratchBlocks) { /* handle missing ScratchBlocks */ } else if (!this.ScratchBlocks.FieldCustom) { /* current stub logic */ }
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -8,7 +8,10 @@ import log from '../lib/log'; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import extensionLibraryContent, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryError, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryLoading, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryMore | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryMore, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryLoadingOB, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryMoreOB, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| galleryErrorOB | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } from '../lib/libraries/extensions/index.jsx'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import extensionTags from '../lib/libraries/tw-extension-tags'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -39,9 +42,57 @@ const translateGalleryItem = (extension, locale) => ({ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: extension.descriptionTranslations[locale] || extension.description | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let cachedGallery = null; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Timeout constant for gallery loading | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Timeout constant for gallery loading | |
| // Timeout before showing the gallery loading state (in ms). | |
| // 750ms is a UX tradeoff: short enough to show feedback on slower connections, | |
| // but long enough that fast responses don't briefly flash a loading indicator. |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The constant GALLERY_TIMEOUT_MS is defined but would be better named as GALLERY_LOADING_TIMEOUT_MS or similar to clarify that this is specifically for the loading indicator timeout, not a hard timeout for the fetch itself. The name could be confused with a timeout for the actual fetch operation.
| // Timeout constant for gallery loading | |
| const GALLERY_TIMEOUT_MS = 750; | |
| // Timeout constant for gallery loading indicator (not the fetch itself) | |
| const GALLERY_LOADING_TIMEOUT_MS = 750; |
Copilot
AI
Jan 19, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment "Common gallery fetcher function to reduce code duplication" on line 48 is misleading because two separate functions fetchLibraryTW and fetchLibraryOB are implemented below, not a single common function. The actual common function is loadGalleryWithTimeout introduced later. Either update this comment to accurately describe the refactoring, or move it to line 138 where the truly common helper is defined.
| // Common gallery fetcher function to reduce code duplication | |
| // Gallery fetcher helpers and caches |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable naming has changed from singular to plural (gallery → galleryTW, galleryOB) but the logic implies these are singular gallery objects, not collections. Consider more descriptive names like turbowarpGallery and omniblocksGallery to be clearer about what these variables represent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Credit keys may not be unique.
Using credit.name as the key (Line 77) may not be unique if multiple credits share the same name, leading to React key warnings.
Apply this diff to ensure unique keys:
- ].map(credit => {
+ ].map((credit, index) => {
if (credit.link) {
return (
<a
href={credit.link}
target="_blank"
rel="noreferrer"
- key={credit.name}
+ key={`${credit.name}-${index}`}
>
{credit.name}
</a>
);
}
- return credit.name;
+ return <span key={`${credit.name}-${index}`}>{credit.name}</span>;
}),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| credits: [ | |
| ...(extension.original || []), | |
| ...(extension.by || []) | |
| ].map(credit => { | |
| if (credit.link) { | |
| return ( | |
| <a | |
| href={credit.link} | |
| target="_blank" | |
| rel="noreferrer" | |
| key={credit.name} | |
| > | |
| {credit.name} | |
| </a> | |
| ); | |
| } | |
| return credit.name; | |
| }), | |
| credits: [ | |
| ...(extension.original || []), | |
| ...(extension.by || []) | |
| ].map((credit, index) => { | |
| if (credit.link) { | |
| return ( | |
| <a | |
| href={credit.link} | |
| target="_blank" | |
| rel="noreferrer" | |
| key={`${credit.name}-${index}`} | |
| > | |
| {credit.name} | |
| </a> | |
| ); | |
| } | |
| return <span key={`${credit.name}-${index}`}>{credit.name}</span>; | |
| }), |
🤖 Prompt for AI Agents
In src/containers/extension-library.jsx around lines 67 to 84, the map uses
credit.name as the React key which may not be unique; change the mapping to use
the map index (or a unique credit id if available) to construct a stable unique
key (e.g. combine credit.name with the index or use credit.id when present) and
apply that key to the returned <a> element (and to non-<a> returns if they are
rendered as list items) so React warnings about duplicate keys are eliminated.
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code duplication between fetchLibraryTW and fetchLibraryOB functions. These functions share almost identical logic except for URLs and tags. Consider creating a shared helper function that accepts the base URL and tag as parameters to follow the DRY principle.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function parameters should be documented with JSDoc to clarify their types and purposes. Add a JSDoc comment explaining that this is a utility to show loading states while preventing indefinite loading indicators.
| // Helper function to handle gallery loading with timeout | |
| // Helper function to handle gallery loading with timeout | |
| /** | |
| * Utility to load a gallery while preventing indefinite loading indicators. | |
| * Starts a timeout that triggers a loading state callback, then runs the provided | |
| * fetch function and routes the result to success or error callbacks as appropriate. | |
| * | |
| * @param {() => Promise<unknown>} fetchFunction - Asynchronous function that fetches the gallery data. | |
| * @param {() => void} timeoutCallback - Invoked when the timeout elapses to show a loading state. | |
| * @param {(gallery: unknown) => void} successCallback - Called with the fetched gallery data on success, | |
| * provided the timeout has not already fired. | |
| * @param {(error: Error) => void} errorCallback - Called with an error if fetching fails before the | |
| * timeout has fired. | |
| */ |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a potential race condition where the timeout might fire between the check at line 148 and the clearTimeout at line 151. While unlikely to cause issues in practice, consider clearing the timeout before checking the flag, or use a more robust cancellation pattern.
| if (!timeoutFired) { | |
| successCallback(gallery); | |
| } | |
| clearTimeout(timeout); | |
| }) | |
| .catch(error => { | |
| if (!timeoutFired) { | |
| log.error(error); | |
| errorCallback(error); | |
| } | |
| clearTimeout(timeout); | |
| clearTimeout(timeout); | |
| if (!timeoutFired) { | |
| successCallback(gallery); | |
| } | |
| }) | |
| .catch(error => { | |
| clearTimeout(timeout); | |
| if (!timeoutFired) { | |
| log.error(error); | |
| errorCallback(error); | |
| } |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error handling in loadGalleryWithTimeout only logs errors but doesn't provide any user feedback mechanism beyond showing the error state. Consider whether error details should be passed to the error callback for potential user display, or if retry logic should be implemented for transient network failures.
| const loadGalleryWithTimeout = (fetchFunction, timeoutCallback, successCallback, errorCallback) => { | |
| let timeoutFired = false; | |
| const timeout = setTimeout(() => { | |
| timeoutFired = true; | |
| timeoutCallback(); | |
| }, GALLERY_TIMEOUT_MS); | |
| fetchFunction() | |
| .then(gallery => { | |
| if (!timeoutFired) { | |
| successCallback(gallery); | |
| } | |
| clearTimeout(timeout); | |
| }) | |
| .catch(error => { | |
| if (!timeoutFired) { | |
| log.error(error); | |
| errorCallback(error); | |
| } | |
| clearTimeout(timeout); | |
| }); | |
| const loadGalleryWithTimeout = ( | |
| fetchFunction, | |
| timeoutCallback, | |
| successCallback, | |
| errorCallback, | |
| maxRetries = 0 | |
| ) => { | |
| let timeoutFired = false; | |
| let attempts = 0; | |
| const timeout = setTimeout(() => { | |
| timeoutFired = true; | |
| timeoutCallback(); | |
| }, GALLERY_TIMEOUT_MS); | |
| const attemptFetch = () => { | |
| attempts += 1; | |
| fetchFunction() | |
| .then(gallery => { | |
| if (!timeoutFired) { | |
| successCallback(gallery); | |
| } | |
| clearTimeout(timeout); | |
| }) | |
| .catch(error => { | |
| if (timeoutFired) { | |
| clearTimeout(timeout); | |
| return; | |
| } | |
| log.error(error); | |
| if (attempts <= maxRetries) { | |
| // Retry for transient failures within the timeout window | |
| attemptFetch(); | |
| return; | |
| } | |
| errorCallback(error); | |
| clearTimeout(timeout); | |
| }); | |
| }; | |
| attemptFetch(); |
ampelectrecuted marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The variable 'library' is reassigned from null to an array, which makes the code flow less clear. Consider initializing it as an array directly instead of starting with null, since it's always assigned to extensionLibraryContent.map(toLibraryItem) before being used.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The render logic has become complex with duplicated patterns for TurboWarp and OmniBlocks galleries. Consider extracting a helper method like addGalleryItems(gallery, error, timedOut, loading, more, errorItem) to reduce duplication and improve maintainability.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The render logic shows galleryMore (TurboWarp gallery header) even when the TurboWarp gallery has loaded successfully. This is inconsistent - galleryMore should only appear when there are items to show. The same pattern exists for the OmniBlocks gallery at line 264 with galleryMoreOB. These 'more' items should typically appear before the list, not when nothing has loaded yet, or the naming is misleading.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A separator is always pushed between the two galleries even when one or both galleries haven't loaded, failed to load, or are still loading. This could result in multiple consecutive separators (one after line 241, another at line 260) appearing in the UI when galleries are in loading/error states, creating a poor user experience.
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The library is unconditionally built and populated, even when neither gallery has loaded. The previous implementation checked if any gallery content existed before building the full library. Consider maintaining this pattern to avoid showing separators when galleries haven't loaded yet.
| library.push('---'); | |
| const locale = this.props.intl.locale; | |
| // Add TurboWarp gallery items | |
| if (this.state.galleryTW) { | |
| library.push(toLibraryItem(galleryMore)); | |
| library.push( | |
| ...this.state.galleryTW | |
| .map(i => translateGalleryItem(i, locale)) | |
| .map(toLibraryItem) | |
| ); | |
| } else if (this.state.galleryTWError) { | |
| library.push(toLibraryItem(galleryError)); | |
| } else if (this.state.galleryTWTimedOut) { | |
| library.push(toLibraryItem(galleryLoading)); | |
| } | |
| // Add separator between galleries | |
| library.push('---'); | |
| // Add OmniBlocks gallery items | |
| if (this.state.galleryOB) { | |
| library.push(toLibraryItem(galleryMoreOB)); | |
| library.push( | |
| ...this.state.galleryOB | |
| .map(i => translateGalleryItem(i, locale)) | |
| .map(toLibraryItem) | |
| ); | |
| } else if (this.state.galleryOBError) { | |
| library.push(toLibraryItem(galleryErrorOB)); | |
| } else if (this.state.galleryOBTimedOut) { | |
| library.push(toLibraryItem(galleryLoadingOB)); | |
| } | |
| const hasTurboWarpGallerySection = this.state.galleryTW || | |
| this.state.galleryTWError || | |
| this.state.galleryTWTimedOut; | |
| const hasOmniBlocksGallerySection = this.state.galleryOB || | |
| this.state.galleryOBError || | |
| this.state.galleryOBTimedOut; | |
| const locale = this.props.intl.locale; | |
| // Only add gallery sections (and separators) if at least one gallery has content/loading/error | |
| if (hasTurboWarpGallerySection || hasOmniBlocksGallerySection) { | |
| // Separator between built-in extensions and first gallery | |
| library.push('---'); | |
| // Add TurboWarp gallery items | |
| if (hasTurboWarpGallerySection) { | |
| if (this.state.galleryTW) { | |
| library.push(toLibraryItem(galleryMore)); | |
| library.push( | |
| ...this.state.galleryTW | |
| .map(i => translateGalleryItem(i, locale)) | |
| .map(toLibraryItem) | |
| ); | |
| } else if (this.state.galleryTWError) { | |
| library.push(toLibraryItem(galleryError)); | |
| } else if (this.state.galleryTWTimedOut) { | |
| library.push(toLibraryItem(galleryLoading)); | |
| } | |
| } | |
| // Add separator between galleries only if both galleries are present in some form | |
| if (hasTurboWarpGallerySection && hasOmniBlocksGallerySection) { | |
| library.push('---'); | |
| } | |
| // Add OmniBlocks gallery items | |
| if (hasOmniBlocksGallerySection) { | |
| if (this.state.galleryOB) { | |
| library.push(toLibraryItem(galleryMoreOB)); | |
| library.push( | |
| ...this.state.galleryOB | |
| .map(i => translateGalleryItem(i, locale)) | |
| .map(toLibraryItem) | |
| ); | |
| } else if (this.state.galleryOBError) { | |
| library.push(toLibraryItem(galleryErrorOB)); | |
| } else if (this.state.galleryOBTimedOut) { | |
| library.push(toLibraryItem(galleryLoadingOB)); | |
| } | |
| } | |
| } |
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The gallery rendering logic always shows both galleries or their loading/error states, which means users will always see both TurboWarp and OmniBlocks sections. Consider adding a configuration option or environment variable to control which galleries are displayed, especially if OmniBlocks wants the flexibility to only show their own gallery in certain deployments.
Copilot
AI
Jan 17, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The render logic has become more complex with the addition of dual galleries. Consider extracting the gallery rendering logic into separate helper methods (e.g., renderTurboWarpGallery, renderOmniblocksGallery) to improve readability and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve the max-len lint failure.
The long inline comments trigger the lint error; consider removing or wrapping them.
🧹 Proposed fix
📝 Committable suggestion
🧰 Tools
🪛 GitHub Check: 🎨 ESLint Results
[failure] 24-24: max-len
This line has a length of 166. Maximum allowed is 120. (max-len)
🤖 Prompt for AI Agents