fix(web): add some missing styling #1257
Conversation
✅ Deploy Preview for reearth-cms ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe changes introduced in this pull request encompass several components across the application. A new icon, 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 using PR comments)
Other keywords and placeholders
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 and nitpick comments (7)
web/src/components/molecules/Asset/Asset/AssetBody/UnzipFileList/index.tsx (2)
85-85: Good enhancement to the tree structure visualization.The
showLineprop update improves the tree's visual structure by explicitly showing leaf icons. This change supports the PR's objective of adding folder and file icons to the asset tree.For improved readability, consider destructuring the
showLineprop:-showLine={{ showLeafIcon: true }} +showLine={({ showLeafIcon: true })}This minor change makes it clearer that we're passing an object to the
showLineprop.
149-153: Good addition of theSwitcherIconstyled component.This new component ensures consistent rendering of icons by preventing unwanted transformations. It's well-placed and follows the convention of defining styled components after the main component.
Consider adding a comment explaining why the
!importantrule is necessary:const SwitcherIcon = styled(Icon)` svg { + /* Override default Tree component styles to ensure consistent icon rendering */ transform: none !important; } `;This comment will help future developers understand the reasoning behind using
!important.web/src/components/molecules/Common/Form/AssetItem/index.tsx (2)
155-165: Improved asset link rendering logicThe updated rendering logic enhances clarity and explicitly handles removed assets. This change aligns with the PR objective of improving the information card on the item edit page.
Consider extracting the link URL to a constant or utility function to improve maintainability:
const getAssetUrl = (workspaceId: string, projectId: string, assetId: string) => `/workspace/${workspaceId}/project/${projectId}/asset/${assetId}`; // Usage <Link to={getAssetUrl(workspaceId, projectId, value)} target="_blank"> {/* ... */} </Link>
Line range hint
252-275: Improved styling for AssetLinkedName and AssetNameThese changes enhance the component's appearance and functionality:
- The
AssetLinkedNamenow uses adisabledprop, which is more intuitive and follows common conventions.- The
AssetNamestyling addresses text overflow issues, aligning with the PR objectives.Consider using CSS custom properties (variables) for the colors to improve maintainability:
const AssetLinkedName = styled(Button)<{ disabled?: boolean }>` --enabled-color: #1890ff; --disabled-color: #00000040; color: ${({ disabled }) => (disabled ? 'var(--disabled-color)' : 'var(--enabled-color)')}; // ... rest of the styles `;This approach allows for easier theming and consistency across the application.
web/src/components/molecules/Asset/AssetListTable/index.tsx (1)
294-299: LGTM! Consider applying consistent formatting.The addition of the
size="small"prop to theDownloadButtoncomponent is a good styling improvement and aligns well with the PR objectives. This change makes the download button consistent with other buttons in the same context.For better code consistency, consider aligning the props vertically:
<DownloadButton displayDefaultIcon size="small" type="link" - selected={props.selectedRows} + selected={props.selectedRows} />This formatting is consistent with other multi-prop components in the file and improves readability.
web/src/components/molecules/Schema/FieldModal/index.tsx (2)
156-160: Consider using a semantic heading element for the modal titleThe change from
<h3>to a styled<p>element (StyledTitle) for the modal title might affect the document's semantic structure. While the styling likely maintains the visual hierarchy, it's generally recommended to use heading elements (<h1>to<h6>) for titles to maintain proper document outline and accessibility.Consider using a styled
<h3>element instead of<p>to maintain semantic meaning while achieving the desired styling:const StyledTitle = styled.h3` /* ... existing styles ... */ margin: 0 20px 0 12px; font-weight: normal; // To match paragraph styling if needed `;This approach would preserve the semantic structure while allowing for custom styling.
395-401: Reconsider the use ofoverflow: autofor the titleThe
StyledTitlecomponent includes anoverflow: autoproperty, which might be unnecessary for a title element. Titles typically don't overflow, and if they do, it might indicate a design issue that should be addressed differently.Consider removing the
overflow: autoproperty:const StyledTitle = styled.p` color: #000000d9; font-size: 16px; margin: 0 20px 0 12px; // Remove or replace the overflow property // overflow: auto; // If text truncation is needed, consider using: // white-space: nowrap; // overflow: hidden; // text-overflow: ellipsis; `;If there's a specific reason for including
overflow: auto, please add a comment explaining the rationale. Otherwise, consider removing it or implementing a text truncation approach if long titles are a concern.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- web/src/components/atoms/Icon/icons.ts (2 hunks)
- web/src/components/molecules/Asset/Asset/AssetBody/UnzipFileList/index.tsx (2 hunks)
- web/src/components/molecules/Asset/AssetListTable/index.tsx (1 hunks)
- web/src/components/molecules/Common/Form/AssetItem/index.tsx (5 hunks)
- web/src/components/molecules/Content/Form/SidebarWrapper.tsx (1 hunks)
- web/src/components/molecules/Schema/FieldModal/index.tsx (2 hunks)
🧰 Additional context used
🔇 Additional comments (9)
web/src/components/molecules/Content/Form/SidebarWrapper.tsx (1)
99-101: Improved styling for better layout and consistencyThe changes to the
DataRowcomponent enhance the layout and spacing:
- The
gapproperty ensures consistent spacing between child elements.- The added
paddingimproves vertical spacing within the row.- The
line-heightproperty ensures consistent vertical alignment of text.These improvements align well with the PR objectives of fixing styling issues and enhancing the overall appearance of components.
web/src/components/molecules/Asset/Asset/AssetBody/UnzipFileList/index.tsx (2)
77-79: Excellent improvement to the folder icons!The dynamic
switcherIconimplementation enhances the user experience by providing clear visual feedback on the expanded state of folders. This change aligns well with the PR's objective of improving visual representation in the asset tree.
Line range hint
1-154: Overall, excellent improvements to the UnzipFileList component!The changes in this file significantly enhance the visual representation and user experience of the asset tree, aligning perfectly with the PR objectives. The dynamic folder icons, improved tree structure visualization, and consistent icon rendering all contribute to a more intuitive and visually appealing interface.
The code is well-structured, follows React best practices, and maintains good readability. Great job on these improvements!
web/src/components/atoms/Icon/icons.ts (2)
36-36: LGTM: New icon import added correctly.The
FolderOpenOutlinedicon is imported correctly from@ant-design/icons. This addition aligns with the PR objective of adding folder icons to improve visual representation in the asset tree.
141-141: LGTM: New icon export added correctly.The
folderOpenicon is correctly exported and mapped to theFolderOpenOutlinedcomponent. This addition complements the new import and fulfills the PR objective of adding folder icons to the asset tree.To ensure this new icon is being used as intended, let's verify its usage in the codebase:
✅ Verification successful
LGTM: New icon export added correctly.
The
folderOpenicon is correctly exported and mapped to theFolderOpenOutlinedcomponent. This addition complements the new import and fulfills the PR objective of adding folder icons to the asset tree. Verified that thefolderOpenicon is utilized inUnzipFileList/index.tsxas intended.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the new folderOpen icon in the codebase. # Test: Search for usage of folderOpen icon echo "Searching for usage of folderOpen icon:" rg --type typescript --type typescriptreact 'folderOpen' # Test: Check for any TODO comments related to folder icons echo "Checking for any TODO comments related to folder icons:" rg --type typescript --type typescriptreact 'TODO.*folder.*icon'Length of output: 403
Script:
#!/bin/bash # Description: Verify the usage of the new folderOpen icon in the codebase. # Test: Search for usage of folderOpen icon in .ts and .tsx files echo "Searching for usage of folderOpen icon in .ts and .tsx files:" rg -g '*.ts' -g '*.tsx' 'folderOpen' # Test: Check for any TODO comments related to folder icons in .ts and .tsx files echo "Checking for any TODO comments related to folder icons in .ts and .tsx files:" rg -g '*.ts' -g '*.tsx' 'TODO.*folder.*icon'Length of output: 612
web/src/components/molecules/Common/Form/AssetItem/index.tsx (3)
151-152: LGTM: Improved icon and asset name displayThe changes here align with the PR objectives:
- The folder icon enhances visual representation.
- Using the
AssetNamecomponent likely improves styling consistency.These modifications contribute to a better user interface.
186-187: LGTM: Improved asset button icon and internationalizationThese changes enhance the user interface and maintainability:
- The link icon better represents the button's purpose of linking an asset.
- Using the
tfunction for the button text ensures proper internationalization support.
228-229: LGTM: Improved AssetButton stylingThe added padding and flex-flow properties enhance the button's appearance and layout. These changes contribute to the PR's objective of fixing styling bugs and improving the overall user interface.
web/src/components/molecules/Schema/FieldModal/index.tsx (1)
Line range hint
1-401: Overall assessment: Styling improvements with minor suggestionsThe changes to the
FieldModalcomponent focus on enhancing the visual styling of the modal title. These modifications improve the appearance without altering the component's functionality. The introduction of theStyledTitlecomponent allows for more precise control over the title's styling.While these changes are generally positive, there are two minor suggestions to consider:
- Using a semantic heading element for the modal title to maintain proper document structure.
- Reconsidering the use of
overflow: autofor the title styling.These suggestions aim to further refine the component's structure and styling without impacting its current improvements.
Overview
This PR fixes some styling bugs.
What I've done
Summary by CodeRabbit
New Features
folderOpen, enhancing the icon set.DownloadButtonto include a smaller size option for improved styling.UnzipFileListcomponent with dynamic icons that reflect the expanded state of tree nodes.Improvements
AssetItemcomponent for better user experience.FieldModaltitle for a more polished appearance.Style