[MOC-179] image alterations #141
Conversation
|
Warning Rate limit exceeded@fitzk has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 23 minutes and 13 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughWalkthroughThe recent changes enhance the modularity and functionality of image upload and editing processes across multiple components. Key updates include making the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
apps/mocksi-lite/content/EditMode/editMode.ts (1)
Line range hint
10-66:
Refactor suggestion: Consider separating modal creation and event handling.The
openImageUploadModalfunction handles both modal creation and event handling. Consider separating these concerns to improve readability and maintainability.function createModalContent(): HTMLElement { const modalContent = document.createElement("div"); modalContent.innerHTML = ` <div id="image-upload-modal" style="display: block; position: fixed; top: 50%; left: 50%; transform: translate(-50%, -50%); background: white; padding: 20px; border: 1px solid #ccc;"> <h2>Upload New Image</h2> <input type="file" id="image-input" accept="image/*"> <button id="upload-button">Upload</button> <button id="cancel-button">Cancel</button> </div> `; return modalContent; } function openImageUploadModal( targetedElement: HTMLImageElement, onChange: (i: number | string, src: string) => void, ) { console.log("targetElement", targetedElement); const modalContainer = document.createElement("div"); document.body.appendChild(modalContainer); const shadowRoot = modalContainer.attachShadow({ mode: "open" }); shadowRoot.appendChild(createModalContent()); const imageInput = shadowRoot.querySelector("#image-input") as HTMLInputElement; const uploadButton = shadowRoot.querySelector("#upload-button") as HTMLButtonElement; const cancelButton = shadowRoot.querySelector("#cancel-button") as HTMLButtonElement; targetedElement.focus(); uploadButton.addEventListener("click", () => { const file = imageInput.files?.[0]; if (file) { convertImageToDataUri(file) .then((newSrc) => { if (targetedElement.srcset) { targetedElement.removeAttribute("srcset"); } targetedElement.src = newSrc; const i = targetedElement.getAttribute("data-mocksi-img"); if (i) { onChange(i, newSrc); console.log("after: ", targetedElement); } }) .catch((error) => { console.error("Error reading file:", error); }); closeImageUploadModal(); } else { console.error("No file selected."); } }); cancelButton.addEventListener("click", closeImageUploadModal); function closeImageUploadModal() { document.body.removeChild(modalContainer); } }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apps/mocksi-lite/content/EditMode/editMode.ts (5 hunks)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx (1 hunks)
- apps/mocksi-lite/content/Toast/EditToast.tsx (10 hunks)
Additional comments not posted (9)
apps/mocksi-lite/content/ListPopup/DemoItem.tsx (1)
73-73: Verify the impact of always enabling the Play button.The button is no longer conditionally disabled based on the
alterationsarray. Ensure that this change does not negatively impact the user experience, especially in scenarios where the button should be disabled.apps/mocksi-lite/content/EditMode/editMode.ts (3)
6-9: Verify the usage of theonChangeparameter inopenImageUploadModal.The
onChangeparameter is a new addition that allows for better integration. Verify that this parameter is used correctly across the codebase and that it effectively handles changes as intended.Verification successful
The
onChangeparameter inopenImageUploadModalis used correctly.The function
setDemoSrcis defined with the appropriate signature and is used to update the image source mapping, ensuring that changes are handled as intended.
apps/mocksi-lite/content/Toast/EditToast.tsx:setDemoSrcis defined and used correctly withopenImageUploadModal.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `onChange` parameter in `openImageUploadModal`. # Test: Search for all calls to `openImageUploadModal` to ensure `onChange` is used correctly. rg --type ts $'openImageUploadModal($_, $_)'Length of output: 45
Script:
#!/bin/bash # Description: Search for all occurrences of `openImageUploadModal` to verify its usage. # Test: Search for all calls to `openImageUploadModal` in the codebase. rg --type ts 'openImageUploadModal'Length of output: 411
Script:
#!/bin/bash # Description: Search for the definition and usage of `setDemoSrc` in `EditToast.tsx`. # Test: Find where `setDemoSrc` is defined and how it is used in `EditToast.tsx`. rg --type ts 'setDemoSrc' apps/mocksi-lite/content/Toast/EditToast.tsx -A 5 -B 5Length of output: 913
119-119: Parameter reordering inapplyEditorfunction.The reordering of
selectedRangeparameters does not affect functionality but improves readability. Ensure that all calls to this function are updated accordingly.Verification successful
All calls to
applyEditorare correctly updated with the new parameter order.The call to
applyEditorinEditToast.tsxmatches the parameter order defined ineditMode.ts. No further updates are needed.
EditToast.tsx:
applyEditor(targetedElement, selection, event.shiftKey, applyAlteration);Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all calls to `applyEditor` are updated with the new parameter order. # Test: Search for all calls to `applyEditor`. rg --type ts $'applyEditor($_, null | $_, $_, $_)'Length of output: 51
Script:
#!/bin/bash # Locate the definition of the `applyEditor` function to understand its parameters. ast-grep --lang typescript --pattern 'function applyEditor($_, $_, $_, $_) { $$$ }' # Search for all calls to `applyEditor` in the codebase to verify parameter order. rg 'applyEditor(' -A 2 --type tsLength of output: 1931
Script:
#!/bin/bash # Correct the search pattern to find all calls to `applyEditor`. rg 'applyEditor\(' -A 2 --type tsLength of output: 460
Script:
#!/bin/bash # Search for the full call to `applyEditor` in `EditToast.tsx` to verify all parameters. rg 'applyEditor\(' -A 5 apps/mocksi-lite/content/Toast/EditToast.tsxLength of output: 175
92-92: Parameter reordering indecorateTextTagfunction.The reordering of
endOffsetandstartOffsetparameters improves readability without affecting functionality. Ensure that all calls to this function are updated accordingly.apps/mocksi-lite/content/Toast/EditToast.tsx (5)
62-62: Use ofuseReffor modal tracking.The
useRefhook is used to track the currently open modal, which enhances handling of double-click events on images. Ensure that this approach effectively manages modal states across different scenarios.Verification successful
Use of
useReffor modal tracking is appropriate.The
modalOpenRefis used to track the index of the currently open modal, ensuring that modal state changes are handled correctly. The implementation checks and updates thecurrentproperty effectively.
- File:
apps/mocksi-lite/content/Toast/EditToast.tsx- Lines: Initialization and conditional updates.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the effectiveness of using `useRef` for tracking open modals. # Test: Search for all references to `modalOpenRef` to ensure it is used correctly. rg --type tsx $'modalOpenRef'Length of output: 56
Script:
#!/bin/bash # Description: Search for all references to `modalOpenRef` in the codebase. # Test: Search for all occurrences of `modalOpenRef` in any file. rg 'modalOpenRef'Length of output: 422
Line range hint
226-244:
Ensure proper cleanup withteardownEditorandresetEditor.Both
teardownEditorandresetEditorfunctions include calls toundoImgEdits. Verify that these functions are invoked correctly to ensure proper cleanup of the component state.Verification successful
Cleanup Functions Verified:
teardownEditorandresetEditorare Invoked CorrectlyThe functions
teardownEditorandresetEditorare defined and invoked within the same file, ensuring proper cleanup of the component state. No issues were found with their usage.
teardownEditoris invoked at line 226 inapps/mocksi-lite/content/Toast/EditToast.tsx.resetEditoris invoked at line 230 in the same file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct invocation of `teardownEditor` and `resetEditor` for cleanup. # Test: Search for all references to `teardownEditor` and `resetEditor`. rg --type tsx $'teardownEditor|resetEditor'Length of output: 72
Script:
#!/bin/bash # Description: Verify the correct invocation of `teardownEditor` and `resetEditor` for cleanup. # Test: Search for all references to `teardownEditor` and `resetEditor` without specifying file type. rg 'teardownEditor|resetEditor'Length of output: 329
125-141: New functionundoImgEditsfor restoring image sources.The
undoImgEditsfunction restores original image sources based on stored mappings. Ensure that this function is called appropriately during the component's lifecycle.Verification successful
Verified Usage of
undoImgEditsFunctionThe
undoImgEditsfunction is appropriately used within the component's lifecycle, specifically in functions that handle resetting or reverting editor states. This ensures that image sources are restored to their original state as intended.
- Calls to
undoImgEditsare found in functions handling modifications and editor resets.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the appropriate use of `undoImgEdits` during the component's lifecycle. # Test: Search for all calls to `undoImgEdits` to ensure it is used correctly. rg --type tsx $'undoImgEdits'Length of output: 56
Script:
#!/bin/bash # Description: Verify the appropriate use of `undoImgEdits` during the component's lifecycle. # Test: Search for all calls to `undoImgEdits` to ensure it is used correctly. rg 'undoImgEdits'Length of output: 227
Script:
#!/bin/bash # Description: Extract context around `undoImgEdits` calls to verify appropriate usage. # Extract 5 lines before and after each call to `undoImgEdits` in `EditToast.tsx`. rg 'undoImgEdits' -A 5 -B 5 apps/mocksi-lite/content/Toast/EditToast.tsxLength of output: 754
162-209: Setup of image data and event listeners insetupEditor.The
setupEditorfunction now gathers image data and sets up event listeners for double-clicks on images. Ensure that this setup is robust and does not introduce performance issues.
112-123: New functionsetDemoSrcfor image source management.The
setDemoSrcfunction updates theimgSrcMapwith demo source information. Ensure that this function integrates well with other parts of the image handling logic.
| const i = targetedElement.getAttribute("data-mocksi-img"); | ||
| if (i) { | ||
| onChange(i, newSrc); | ||
| console.log("after: ", targetedElement); |
| } | ||
| for (let i = 0; i < images.length; i++) { | ||
| const image = images[i]; | ||
| console.log(image); |
| }); | ||
| } | ||
|
|
||
| function undoImgEdits() { |
There was a problem hiding this comment.
Just Couple edits:
- Early Resolve: Moved resolve() to execute immediately after checking the length of images, ensuring that the promise is resolved right away if there are no images.
- for...of Loop: Refactored the loop to use for...of with Array.from(images).entries(). This enhances clarity by directly iterating over the images collection with access to both the index and the image in each iteration.
- Type Annotation: Explicitly typed images as HTMLCollectionOf to ensure strong typing throughout the function.
- Optional Chaining (?.): Added optional chaining when accessing originalSrc to avoid potential runtime errors if imgSrcMap[i] is undefined.
Here's how the final code should look like:
function undoImgEdits(): Promise<void> {
return new Promise<void>((resolve) => {
const images: HTMLCollectionOf<HTMLImageElement> = document.images;
if (images.length === 0) {
resolve();
return;
}
for (const [i, image] of Array.from(images).entries()) {
console.debug(image);
const src: string | undefined = imgSrcMap[i]?.originalSrc;
if (src) {
image.src = src;
}
}
resolve();
});
}(hope you don't mind ChatGPT typing the explanation above 😅 )
| @@ -125,18 +159,72 @@ const EditToast = ({ initialReadOnlyState }: EditToastProps) => { | |||
| applyReadOnlyMode(); | |||
| } | |||
|
|
|||
| document.body.addEventListener("dblclick", onDoubleClickText); | |||
| const images = document.images; | |||
There was a problem hiding this comment.
I’ve refactored the code snippet for improved readability and maintainability. The main changes include:
- for...of Loop: Replaced the for loop with a for...of loop using Array.from(images).entries(). This makes the iteration more expressive and eliminates the need to manually manage the index.
- DRY Principle: Combined the duplicated dblclick event listeners into a single function to avoid redundancy and make the code easier to maintain.
- Type Annotation: Maintained explicit typing for tempImageSrcMap to ensure the structure of the record is clear and strongly typed.
The final code should look like this:
const images: HTMLCollectionOf<HTMLImageElement> = document.images;
const tempImageSrcMap: Record<
number,
{ demoSrc: string; index: string; originalSrc: string }
> = {};
// Function to handle the dblclick event
const handleDblClick = (image: HTMLImageElement, i: number) => (event: Event) => {
event.stopPropagation();
if (modalOpenRef.current !== i) {
openImageUploadModal(image, setDemoSrc);
modalOpenRef.current = i;
}
};
for (const [i, image] of Array.from(images).entries()) {
if (image.style.display === "none" && !image.checkVisibility()) {
continue; // Early return to skip the current iteration
}
tempImageSrcMap[i] = {
demoSrc: "",
index: i.toString(),
originalSrc: image.src,
};
image.setAttribute("data-mocksi-img", i.toString());
image.setAttribute("listener", "true");
const parent = image.parentNode;
if (parent) {
parent.addEventListener("dblclick", handleDblClick(image, i), false);
}
image.addEventListener("dblclick", handleDblClick(image, i), false);
}There was a problem hiding this comment.
(and yes another ChatGPT-assisted suggestion 😅 )
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
apps/mocksi-lite/content/EditMode/editMode.ts (1)
Line range hint
22-22:
Consider removing or replacingconsole.debug.The use of
console.debugmight not be suitable for production code. Consider using a logging library or removing it before finalizing the PR.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (8)
- apps/mocksi-lite/content/EditMode/editMode.ts (2 hunks)
- apps/mocksi-lite/content/Toast/EditToast.tsx (9 hunks)
- apps/mocksi-lite/content/useImages.test.tsx (1 hunks)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
- apps/mocksi-lite/package.json (2 hunks)
- apps/mocksi-lite/tsconfig.json (1 hunks)
- apps/mocksi-lite/vitest.config.ts (1 hunks)
- apps/mocksi-lite/vitest.init.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/mocksi-lite/vitest.config.ts
Files skipped from review as they are similar to previous changes (1)
- apps/mocksi-lite/content/Toast/EditToast.tsx
Additional comments not posted (8)
apps/mocksi-lite/tsconfig.json (2)
20-20: Enhanced type support for testing libraries.The addition of
vitest/globalsand@testing-library/jest-domto thetypesfield improves type support for testing, which is a positive change.
22-22: Ensure exclusion of test files is intentional.The change to exclude both
.tsand.tsxtest files is appropriate if you intend to exclude all test files from compilation. Verify that this aligns with your project's testing strategy.Verification successful
Exclusion of test files is intentional and appropriate.
The exclusion pattern in
tsconfig.jsoncorrectly targets.test.tsand.test.tsxfiles, aligning with the project's strategy to exclude test files from compilation. Configuration and setup files with "test" in their names do not need to be excluded.
- The exclusion pattern
./**/*.test.tsx?is correctly applied.- No additional test files require exclusion.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Ensure that the exclusion of `.ts` and `.tsx` test files is intentional. # Test: List all test files to confirm exclusion is as intended. fd --extension ts --extension tsx --exec echoLength of output: 3265
Script:
#!/bin/bash # Description: Check for any test files with extensions other than `.test.ts` or `.test.tsx`. # Test: List files that contain 'test' in their name but do not end with `.test.ts` or `.test.tsx`. fd --type f --exec echo | grep 'test' | grep -v '\.test\.ts$' | grep -v '\.test\.tsx$'Length of output: 306
apps/mocksi-lite/vitest.init.ts (2)
1-6: Proper setup for testing with JSDOM and Vitest.The imports and setup for using JSDOM and Vitest are correctly configured, which is essential for testing components that interact with the DOM.
8-19: Verify Chrome API mock implementations.Ensure that the mock implementations for the Chrome API accurately reflect the expected behavior in your tests. This is crucial for reliable test results.
apps/mocksi-lite/package.json (2)
16-35: Improved testing capabilities with new dependencies.The addition of testing libraries and updates to type definitions enhance the project's testing framework, which is a positive change.
46-46: Verify the test script configuration.Ensure that the
vitest.config.tsfile is correctly set up and that the test script functions as expected with the new configuration.Verification successful
Test script configuration appears to be correctly set up.
The
vitest.config.tsfile(s) contain valid configurations forvitest, specifyingjsdomas the test environment. Ensure that the intended configuration is being used for your tests. If there are specific concerns about which configuration should be applied, further manual verification may be needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence and correctness of the `vitest.config.ts` file. # Test: Check for the presence of the `vitest.config.ts` file. fd --type f --name 'vitest.config.ts' --exec catLength of output: 299
Script:
#!/bin/bash # Description: Verify the existence and correctness of the `vitest.config.ts` file. # Test: Check for the presence of the `vitest.config.ts` file. fd --type f 'vitest.config.ts' --exec catLength of output: 463
apps/mocksi-lite/content/useImages.test.tsx (1)
1-44: Basic test structure looks good.The initial setup and basic tests for the
useImageshook are well-structured. They provide a foundation for further expansion.apps/mocksi-lite/content/EditMode/editMode.ts (1)
Line range hint
10-37:
Enhancements to modularity and usability are well-implemented.The changes to
decorateTextTagandapplyEditorfunctions improve the modularity and usability of the code.
| document.body.innerHTML = ""; | ||
| }); | ||
|
|
||
| // TODO: expand on or remove test |
There was a problem hiding this comment.
Consider expanding the test coverage.
The TODO comment suggests that the test case is minimal. Consider expanding it to cover additional scenarios, such as error handling and edge cases.
Would you like assistance in expanding the test cases or opening a GitHub issue to track this task?
| reader.onerror = reject; | ||
| reader.readAsDataURL(file); | ||
| }); | ||
| } | ||
|
|
||
| async function getStoredEdits() { | ||
| const storage = await chrome.storage.local.get("mocksi-images"); | ||
| const storedImages = storage["mocksi-images"]; | ||
| const url = new URL(document.location.href); | ||
| const editsForHostname = storedImages?.[url.hostname]; | ||
| if (editsForHostname) { | ||
| console.debug(`no stored images found for ${url.hostname}`); | ||
| return; | ||
| } | ||
| return editsForHostname; | ||
| } | ||
|
|
||
| function undoImagesEdits() { | ||
| return new Promise<void>((resolve) => { | ||
| const images = document.images; | ||
| if (!images.length) { | ||
| resolve(); | ||
| return; | ||
| } | ||
| for (let i = 0; i < images.length; i++) { | ||
| const image = images[i]; | ||
| const edit = edits[i]; | ||
| if (edit) { | ||
| const src = edits[i].originalSrc; | ||
| if (src) { | ||
| // image.replaceWith(image.cloneNode(true)) TODO: look into using this | ||
| image.src = edit.originalSrc; | ||
| } | ||
| } | ||
| } | ||
| resolve(); | ||
| }); | ||
| } | ||
|
|
||
| useEffect(() => { | ||
| const images = document.images; | ||
| if (images.length) { | ||
| console.error("No images found in document"); | ||
| return; | ||
| } | ||
| // use to remove all event listeners on unmount | ||
| const { signal } = abortControllerRef.current; | ||
| if (!signal) { | ||
| console.debug("abort controller undefined"); | ||
| } | ||
| // add data attribute and double click handlers | ||
| for (let i = 0; i < images.length; i++) { | ||
| const image = images[i]; | ||
|
|
||
| // only edit visible image elements | ||
| if (image.checkVisibility()) { | ||
| image.setAttribute("data-mocksi-img", i.toString()); | ||
| image.setAttribute("listener", "true"); | ||
|
|
||
| const parent = image.parentNode; | ||
|
|
||
| const handleDoubleClick: EventListener = (event) => { | ||
| event.stopPropagation(); | ||
| if (uploadModalOpenRef.current !== i) { | ||
| function setDemoSrc(i: number, demoSrc: string) { | ||
| setEdits((prevState) => { | ||
| return { | ||
| ...prevState, | ||
| [i]: { | ||
| ...prevState[i], | ||
| demoSrc, | ||
| }, | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| openImageUploadModal(image, setDemoSrc); | ||
| uploadModalOpenRef.current = i; | ||
| } | ||
| }; | ||
|
|
||
| parent?.addEventListener("dblclick", handleDoubleClick, { signal }); | ||
| image.addEventListener("dblclick", handleDoubleClick, { signal }); | ||
| } | ||
| } | ||
|
|
||
| return () => { | ||
| if (abortControllerRef.current) { | ||
| abortControllerRef.current.abort(); | ||
| } | ||
| }; | ||
| }, [openImageUploadModal]); | ||
|
|
||
| return { | ||
| edits, | ||
| getStoredEdits, | ||
| setEdits, | ||
| storeEdits, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Consider enhancing error handling and performance.
While the useImages hook is well-structured, consider the following improvements:
- Enhance error handling, especially in asynchronous operations like
storeEditsandgetStoredEdits. - Optimize performance by minimizing DOM manipulations, such as creating and removing elements in
openImageUploadModal.
Overall, the hook provides essential functionalities for image management.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx (1 hunks)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/mocksi-lite/content/useImages.tsx
Additional comments not posted (1)
apps/mocksi-lite/content/ListPopup/DemoItem.tsx (1)
69-69: Verify the always-enabledhandlePlaybutton behavior.The
ButtonforhandlePlayis now always enabled. Ensure this change aligns with the intended user experience and does not lead to unexpected behavior, especially ifalterationsis expected to influence the button's state.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
Files selected for processing (4)
- apps/mocksi-lite/content/useImages.test.tsx (1 hunks)
- apps/mocksi-lite/package.json (3 hunks)
- apps/mocksi-lite/tsconfig.json (1 hunks)
- apps/mocksi-lite/vitest.init.ts (1 hunks)
Files skipped from review as they are similar to previous changes (4)
- apps/mocksi-lite/content/useImages.test.tsx
- apps/mocksi-lite/package.json
- apps/mocksi-lite/tsconfig.json
- apps/mocksi-lite/vitest.init.ts
| const jsdom = new JSDOM("<!doctype html><html><body></body></html>"); | ||
| const { window } = jsdom; | ||
|
|
||
| global.document = window.document; |
There was a problem hiding this comment.
I wasn't sure if everything shares one Vite config or not so I just added one to the apps/mocksi-lite scope
There was a problem hiding this comment.
Afaik this is correct, each app/package has its own vitest with its own Vite config. Duplication sometimes is a good thing 😅
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/mocksi-lite/content/useImages.tsx
| // const result = await chrome.storage.local.get([MOCKSI_RECORDING_ID]); | ||
| // const { recordingId } = result[MOCKSI_RECORDING_ID]; | ||
|
|
||
| await chrome.storage.local.set({ |
There was a problem hiding this comment.
Should we not store these as part of alterations rather than a separate entry? Alterations have a type field and I'm pretty sure Reactor will handle the images cc: @jonathankap
| }); | ||
| } | ||
|
|
||
| async function applyEdits() { |
There was a problem hiding this comment.
Maybe add a comment here just we can keep track, but this would eventually be replaced by Reactor I believe
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/mocksi-lite/content/Toast/EditToast.tsx (8 hunks)
- apps/mocksi-lite/content/Toast/PlayToast.tsx (6 hunks)
- apps/mocksi-lite/content/useImages.test.tsx (1 hunks)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
- apps/mocksi-lite/utils.ts (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- apps/mocksi-lite/content/Toast/EditToast.tsx
- apps/mocksi-lite/content/useImages.test.tsx
- apps/mocksi-lite/content/useImages.tsx
Additional comments not posted (5)
apps/mocksi-lite/content/Toast/PlayToast.tsx (4)
16-16: Import ofuseImagesis appropriate.The import statement for
useImagesis correctly added and aligns with the component's functionality enhancements.
59-59: Application of image edits is well-placed.The call to
images.applyEdits()within theuseEffecthook ensures that image edits are applied after loading previous modifications, enhancing functionality.
68-68: DOM setup for image editing is appropriate.The inclusion of
images.setupDom()in thehandleEditfunction is a logical enhancement for preparing the DOM for editing.
80-80: Reversion of image edits is correctly implemented.The call to
images.undoEdits()in thehandleStopfunction ensures that any image edits made during playback are reverted, maintaining state integrity.apps/mocksi-lite/utils.ts (1)
237-239: Early exit for empty alterations is a good addition.The early exit condition in
loadPreviousModificationsprevents unnecessary processing and potential errors whenalterationsis empty or undefined.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx (3 hunks)
- apps/mocksi-lite/content/ListPopup/index.tsx (1 hunks)
- apps/mocksi-lite/content/Toast/EditToast.tsx (7 hunks)
- apps/mocksi-lite/content/Toast/PlayToast.tsx (5 hunks)
- apps/mocksi-lite/content/useImages.test.tsx (1 hunks)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
- apps/mocksi-lite/utils.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- apps/mocksi-lite/content/ListPopup/index.tsx
Files skipped from review as they are similar to previous changes (6)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx
- apps/mocksi-lite/content/Toast/EditToast.tsx
- apps/mocksi-lite/content/Toast/PlayToast.tsx
- apps/mocksi-lite/content/useImages.test.tsx
- apps/mocksi-lite/content/useImages.tsx
- apps/mocksi-lite/utils.ts
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx (3 hunks)
- apps/mocksi-lite/content/ListPopup/index.tsx (5 hunks)
- apps/mocksi-lite/content/Toast/PlayToast.tsx (5 hunks)
- apps/mocksi-lite/content/useImages.test.tsx (1 hunks)
- apps/mocksi-lite/content/useImages.tsx (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/mocksi-lite/content/ListPopup/DemoItem.tsx
- apps/mocksi-lite/content/ListPopup/index.tsx
- apps/mocksi-lite/content/Toast/PlayToast.tsx
- apps/mocksi-lite/content/useImages.test.tsx
- apps/mocksi-lite/content/useImages.tsx
| document.body.innerHTML = ""; | ||
| }); | ||
|
|
||
| // TODO: expand on or remove test |
apps/mocksi-lite/package.json
Outdated
| @@ -1,5 +1,6 @@ | |||
| { | |||
| "dependencies": { | |||
| "-": "^0.0.1", | |||
There was a problem hiding this comment.
What's this dependency?
Just curious
There was a problem hiding this comment.
oops that's def not supposed to be there good catch!
| "resolveJsonModule": true, | ||
| "strict": true, | ||
| "sourceMap": false, | ||
| "moduleResolution": "node", |
There was a problem hiding this comment.
sigh, these re-ordering diffs are super distracting. Can't wait to remove biome 😄
There was a problem hiding this comment.
they are, I try to keep them at a minimum believe it or not haha
| const jsdom = new JSDOM("<!doctype html><html><body></body></html>"); | ||
| const { window } = jsdom; | ||
|
|
||
| global.document = window.document; |
There was a problem hiding this comment.
Afaik this is correct, each app/package has its own vitest with its own Vite config. Duplication sometimes is a good thing 😅
pnpm-lock.yaml
Outdated
| @@ -33,6 +33,9 @@ importers: | |||
|
|
|||
| apps/mocksi-lite: | |||
| dependencies: | |||
| '-': | |||
There was a problem hiding this comment.
Again, just wondering about this dependency.
Summary by CodeRabbit
New Features
EditToastvia double-click events.Bug Fixes