feat: updated the ui of editor page #67
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces a new "starred" route and a corresponding Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant StarredPage
participant Cookies
participant ReduxStore
User->>Router: Navigate to '/starred'
Router->>StarredPage: Render StarredPage component
StarredPage->>Cookies: Retrieve authentication token
alt Token Missing
StarredPage->>Router: Redirect to '/login'
else Token Valid
StarredPage->>ReduxStore: Dispatch action to show folders
StarredPage->>ReduxStore: Fetch explorer data if not loaded
ReduxStore-->>StarredPage: Return data and folder order
StarredPage->>User: Display favorite folders (or message if empty)
end
Poem
🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
| @@ -146,12 +145,12 @@ const Sidebar: FC<IProps> = ({ isProfilePage, isWorkspaceSettingsPage }) => { | |||
| )} | |||
|
|
|||
| <div> | |||
| <div ref={favFoldersRef}> | |||
| {/* <div ref={favFoldersRef}> | |||
There was a problem hiding this comment.
removed that component that shows on click and replace with route
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
apps/portal/components/pagesComponents/_imagesScreen/components/folderItem/FolderItem.module.scss (1)
18-18: Missing semicolon in CSS propertyThe height property is missing a semicolon at the end, which is inconsistent with the coding style used throughout the rest of the file.
- height:58px + height: 58px;apps/portal/components/pagesComponents/_imagesScreen/components/folderItem/FolderItem.tsx (1)
360-360: Redundant styling in both CSS and inline stylesThe height of 58px is defined both in the CSS module (FolderItem.module.scss) and as an inline style here, creating redundancy. This can lead to confusion about which style takes precedence and makes maintenance more difficult.
Choose one approach consistently:
- style={{ height: '58px' }} + className={styles.mainWrapper}And keep the height definition only in the CSS module.
apps/portal/pagesScss/media/Images.module.scss (1)
51-56: New foldersFavourites class needs formatting fixesThe new class has proper flexbox layout properties, but there are some formatting issues:
- Missing semicolon after the last property
- Inconsistent spacing around the colon in property declarations
.foldersFavourites{ display: flex; flex-wrap: wrap; gap: 18px; - margin-left: 32px + margin-left: 32px; }apps/portal/components/containers/dashboardLayout/Sidebar/Sidebar.tsx (1)
147-153: Transitioned from sidebar section to dedicated page.The FavFoldersSidebarSection has been commented out as the functionality has been moved to a dedicated "Starred" page. Consider removing this commented code if the new implementation is confirmed to be working correctly.
- {/* <div ref={favFoldersRef}> - <FavFoldersSidebarSection - visible={showFavoriteFolders} - setVisible={setShowFavoriteFolders} - /> - </div>*/}apps/portal/pages/media/starred.tsx (1)
121-137: Add explicit height to the folder item for consistent UI.The component appears to be missing an explicit height for folder items, which could lead to inconsistent UI display. Based on the PR description about UI updates, consider adding a consistent height to the folder items.
<div className="tw-h-[calc(100vh-206px)] tw-overflow-y-auto"> - <div className={styles.foldersFavourites} id="scrollableDiv"> + <div className={`${styles.foldersFavourites} tw-h-58px`} id="scrollableDiv" style={{ height: '58px' }}> {folderData .filter((v) => isFavorite(v)) .map((folder: IDbFolderData) => (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/portal/components/_routes.ts(1 hunks)apps/portal/components/containers/dashboardLayout/Sidebar/Sidebar.tsx(4 hunks)apps/portal/components/pagesComponents/_imagesScreen/components/folderItem/FolderItem.module.scss(1 hunks)apps/portal/components/pagesComponents/_imagesScreen/components/folderItem/FolderItem.tsx(1 hunks)apps/portal/misc/menuItems.tsx(1 hunks)apps/portal/pages/media/starred.tsx(1 hunks)apps/portal/pagesScss/media/Images.module.scss(1 hunks)
🔇 Additional comments (8)
apps/portal/components/_routes.ts (1)
25-25: LGTM: New route for starred items added correctlyThe new
starredroute is properly integrated within thepanelRoutesobject, following the established pattern. This addition supports the new UI feature for accessing starred items.apps/portal/misc/menuItems.tsx (1)
49-49: Route properly updated for the 'favFolders' menu item.The route for the "Starred" menu item is now correctly pointing to the new starred route instead of an empty string. This change enables proper navigation when the user clicks on this menu item.
apps/portal/components/containers/dashboardLayout/Sidebar/Sidebar.tsx (3)
1-1: Removed unused import.The
useCallbackimport has been removed as it's no longer needed after the favorite folders functionality changes.
83-83: Simplified isActive condition.The
isActivefunction now only excludes the 'back' item type, allowing the 'favFolders' item to be active when its route matches the current path.
112-114: Enabled proper navigation for favFolders.Commenting out this special case ensures that the 'favFolders' menu item is now wrapped in a
<Link>component like other menu items, enabling proper navigation.apps/portal/pages/media/starred.tsx (3)
58-58: Incomplete implementation noted by TODO comment.There's a commented out line with a TODO comment. Consider implementing the explorer data listener or removing this comment if it's no longer applicable.
126-134: Empty callback functions in FolderItem.The
onClickandonDropprops are set to empty functions. Consider implementing actual functionality or document why these are intentionally no-ops.<FolderItem key={folder.id} folder={folder} - onClick={() => void 0} + onClick={() => {/* TODO: Implement folder click behavior */}} - onDrop={() => void 0} + onDrop={() => {/* TODO: Implement folder drop behavior */}} setLoading={(loadingState) => setLoading(loadingState)} isFavorite={true} canEdit={false} />
1-163: Well-implemented StarredPage component.The overall structure of the StarredPage component is well-designed. It fetches necessary data, handles authentication, and provides appropriate UI for displaying favorite folders with a good empty state handling.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Sir @evereq Please verify this if this looks good let me know I'll fix this on extension also but first I need your approval |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/portal/pages/media/starred.tsx (1)
123-124:⚠️ Potential issuePotential null or undefined values in isFavorite invocation.
When filtering folders with
isFavorite, the function might returnundefinedinstead of a boolean as identified in a previous comment. This could lead to unexpected filtering behavior.{Array.isArray(folderData) && - folderData.filter((v) => isFavorite(v)).length ? ( + folderData.filter((v) => !!isFavorite(v)).length ? (This is a temporary fix while you also fix the
isFavoritefunction itself as mentioned in the earlier comment.
🧹 Nitpick comments (5)
apps/portal/pages/media/starred.tsx (5)
58-58: Remove or implement commented code.There's a commented code line with a TODO that might need to be addressed before merging this PR. Either implement the functionality, remove the comment, or add a more detailed explanation about what needs to be fixed.
127-128: Repeated filtering logic.The same filtering logic is applied twice - once to check if there are any favorite folders and again when rendering them. This is inefficient.
- {folderData - .filter((v) => isFavorite(v)) - .map((folder: IDbFolderData) => ( + {folderData.filter((v) => !!isFavorite(v)).map((folder: IDbFolderData) => (Consider storing the filtered results in a variable to avoid duplicate processing:
const favoriteFolderItems = folderData.filter((v) => !!isFavorite(v)); // Then in your JSX: {Array.isArray(folderData) && favoriteFolderItems.length ? ( // ... {favoriteFolderItems.map((folder: IDbFolderData) => ( // ... ))}
132-133: Empty click and drop handlers.The
onClickandonDrophandlers are set to() => void 0, which effectively does nothing. If these handlers are intentionally meant to be no-ops, consider adding a comment explaining why or remove them if they're not needed.- onClick={() => void 0} - onDrop={() => void 0} + // No action needed when clicking or dropping on favorite folders + onClick={() => {}} + onDrop={() => {}}
151-151: Inconsistent spelling of "Favourite".The word "Favourite" is used here, but "Favorite" is used elsewhere in the component (line 114). Choose one spelling consistently throughout the application.
- No Favourite Folders found{' '} + No Favorite Folders found{' '}
124-124: Hard-coded height calculations.The component uses hard-coded height calculations in Tailwind classes (
tw-h-[calc(100vh-206px)]). This might not be responsive on all screen sizes.Consider using a more flexible layout approach or ensuring these values adjust for different screen sizes.
Also applies to: 142-142
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/portal/pages/media/starred.tsx(1 hunks)
🔇 Additional comments (1)
apps/portal/pages/media/starred.tsx (1)
88-97: Incomplete boolean return in isFavorite function.The isFavorite function doesn't return a boolean when favoriteFolders?.images is falsy.
const isFavorite = useCallback( (folder: IDbFolderData) => { if (favoriteFolders?.images) { return parseCollectionToArray(favoriteFolders.images).some( (x) => x.id === folder.id, ); } + return false; }, [favoriteFolders], );
| @@ -81,7 +80,7 @@ const Sidebar: FC<IProps> = ({ isProfilePage, isWorkspaceSettingsPage }) => { | |||
| }; | |||
|
|
|||
| const isActive = (item: IMainMenuItem) => { | |||
| if (item.type === 'favFolders' || item.type === 'back') return false; | |||
| if (item.type === 'back') return false; | |||
There was a problem hiding this comment.
@Anishali2 why you removed item.type === 'favFolders' ??
|
@Anishali2 screenshots "before" and "after" completely does not make any sense as they show totally different states / data etc. I also can't get sense of code changes, sorry... Please explain everything you do in this PR better and show how it was worked before and how it will work after your changes. Also, that feature seems called "Favorite Folders" (or just "Favorites"), but you used term "starred", not good |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Brother @paradoxe35 can you please review this PR first and approve first.
|
Sure @Anishali2 |
Before
After
Summary by CodeRabbit