Fix: Preserve category order on rename#31
Conversation
Previously, renaming a category would cause it to move to the bottom of the category list. This was due to the way the underlying JavaScript object storing the categories was manipulated, where new keys are often added to the end. This commit modifies the `handleRenameCategory` function in `useCategoryManagement.tsx` to ensure that the original order of categories is maintained. It achieves this by iterating over the existing keys in order and rebuilding the categories object, replacing the old category name with the new one at its original position. Additionally, after renaming, the `selectedTab` state is now updated to the new category name, ensuring the UI remains focused on the renamed category. I have verified that categories now maintain their correct order after being renamed, whether they are at the beginning, middle, or end of the list.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @jricardo27, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini here, providing a summary of this pull request by @jricardo27.
This PR addresses an issue where renaming a category would cause it to lose its original position and move to the end of the category list. This happened because the underlying JavaScript object storing the categories was being manipulated in a way that didn't preserve key order reliably across operations like deletion and re-addition.
The fix involves modifying the handleRenameCategory function to explicitly iterate over the existing category keys in their current order and build a new object, inserting the renamed category at its original position in the iteration. This ensures the order is maintained. Additionally, the selectedTab state is updated to the new category name after a successful rename, keeping the UI focused correctly.
Highlights
- Fix Category Order on Rename: The core issue of categories moving to the end of the list after renaming is fixed by changing how the
savedFeaturesstate object is updated inhandleRenameCategory. Instead of deleting and re-adding the key, the function now iterates over the existing keys and constructs a new object, placing the renamed category in its original sequential position. - Update Selected Tab: After a category is successfully renamed, the
selectedTabstate is updated to the new category name. This ensures the UI remains focused on the category that was just renamed. - Add Standalone Test Script: A new file,
category_rename_test.js, has been added. This is a simplified, self-contained script that mocks the relevant state and functions (setSavedFeatures,setSelectedTab,handleAddCategory,handleRenameCategory) to specifically test and verify that the category renaming logic correctly preserves order when renaming categories at different positions (middle, first, last). - Dependency Updates: The
package.jsonandpackage-lock.jsonfiles have been updated. This includes updating the@types/nodedependency and addingts-nodeand several related development dependencies, likely to support the new test script or other development workflows.
Changelog
Click here to see the changelog
- category_rename_test.js
- Added a new standalone JavaScript test script to verify the category renaming logic.
- Includes mock state and simplified functions mirroring the component's logic.
- Tests renaming categories at the beginning, middle, and end of a manually reordered list to ensure order preservation.
- package-lock.json
- Updated
@types/nodefrom^22.15.14to^22.15.19. - Added
ts-nodeand several related development dependencies (@cspotcode/source-map-support,@jridgewell/trace-mapping,@tsconfig/node10,@tsconfig/node12,@tsconfig/node14,@tsconfig/node16,acorn-walk,arg,create-require,diff,make-error,v8-compile-cache-lib,yn).
- Updated
- package.json
- Updated
@types/nodefrom^22.15.14to^22.15.19. - Added
ts-nodeas a dev dependency (^10.9.2).
- Updated
- src/components/SavedFeaturesDrawer/hooks/useCategoryManagement.tsx
- Modified the
handleRenameCategoryfunction (lines 47-56) to iterate over the existing keys of theprevstate object and build a new object, ensuring the renamed category is placed at its original position in the iteration order. - Added a call to
setSelectedTab(newName)(line 58) after the state update to automatically switch to the newly renamed category tab.
- Modified the
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Object keys may roam,
But iteration holds the line,
Order is restored.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Corrected an issue in `CategoryContextMenu.tsx` where a `React.Fragment` was being passed as a direct child to MUI's `Menu` component under certain conditions. This was causing the console warning: "MUI: The Menu component doesn't accept a Fragment as a child." The fix involves refactoring the conditional rendering logic to return an array of `MenuItem` components directly, instead of wrapping them in a `React.Fragment`. This aligns with MUI's expectation for the children of the `Menu` component and resolves the console warning. The functionality of the category context menu remains unchanged.
|
/gemini review |
There was a problem hiding this comment.
Code Review
The pull request addresses an important issue where renaming categories would disrupt their order. The provided solution appears to correctly maintain the order by iterating over existing keys. The addition of a test case is also a great way to ensure the fix works as expected. Overall, this is a well-structured and valuable contribution.
Summary of Findings
- Potential Category Overwrite: The
handleRenameCategoryfunction should include a check to ensure that thenewNamedoes not already exist as a category to prevent accidental overwrites. - Logging Enhancement: Adding a logging statement within the
handleRenameCategoryfunction would provide valuable information for debugging and monitoring purposes.
Merge Readiness
The pull request effectively addresses the category renaming issue and includes a test case to verify the fix. While there are a few minor suggestions for improvement, the code is generally well-structured and ready for merging after addressing the medium severity issue.
| console.log(`Category "${oldName}" renamed to "${newName}".`); | ||
| } else { | ||
| console.error("Invalid rename operation:", { oldName, newName, DEFAULT_CATEGORY }); | ||
| } |
There was a problem hiding this comment.
Consider adding an else block to the initial if statement on line 48 to provide more informative feedback when the rename operation is invalid. This could help in debugging test failures or unexpected behavior.
} else {
console.error("Rename test setup invalid:", { oldName, newName, DEFAULT_CATEGORY });
}This commit enhances the `handleRenameCategory` function in `useCategoryManagement.tsx` to address several issues:
1. **Data Loss Prevention:**
- Prevents renaming a category to an existing category name if that name belongs to a *different* category.
- An alert is shown to you, and the operation is aborted.
- Checks are also in place to prevent renaming the default "all" category or renaming any category *to* "all".
2. **No-Op Rename Handling:**
- If a category is "renamed" to its current name, the function now returns early, preventing unnecessary state updates and potential side effects.
These changes ensure that category renaming is safer, more efficient, and provides better feedback to you in conflicting scenarios. The order of categories is preserved during successful renames, and the selected tab is correctly updated.
1 participant
Previously, renaming a category would cause it to move to the bottom of the category list. This was due to the way the underlying JavaScript object storing the categories was manipulated, where new keys are often added to the end.
This commit modifies the
handleRenameCategoryfunction inuseCategoryManagement.tsxto ensure that the original order of categories is maintained. It achieves this by iterating over the existing keys in order and rebuilding the categories object, replacing the old category name with the new one at its original position.Additionally, after renaming, the
selectedTabstate is now updated to the new category name, ensuring the UI remains focused on the renamed category.I have verified that categories now maintain their correct order after being renamed, whether they are at the beginning, middle, or end of the list.