-
Notifications
You must be signed in to change notification settings - Fork 101
Ks 025 sep 25 figma icons 2047 #2222
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
Changes from 4 commits
df614d2
f7ad4d7
95b4cca
2a8c37b
a86dfef
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -17,7 +17,7 @@ import { | |||||
| Tooltip, | ||||||
| } from "@mui/material"; | ||||||
| import ArrowBackIcon from "@mui/icons-material/ArrowBack"; | ||||||
| import CheckCircleIcon from "@mui/icons-material/CheckCircle"; | ||||||
| import { ReactComponent as CheckCircleIcon } from "../../assets/icons/check-circle-white.svg"; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Import CheckCircle icon from the correct location.
-import { ReactComponent as CheckCircleIcon } from "../../assets/icons/check-circle-white.svg";
+import { ReactComponent as CheckCircleIcon } from "../../../assets/icons/check-circle-white.svg";📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||
| import { ReactComponent as GreyCircleInfoIcon } from "../../assets/icons/info-circle-grey.svg"; | ||||||
| import { ReactComponent as BlueInfoIcon } from "../../assets/icons/info-circle-blue.svg"; | ||||||
| import DownloadIcon from "@mui/icons-material/Download"; | ||||||
|
|
@@ -818,7 +818,7 @@ export default function BiasAndFairnessResultsPage() { | |||||
| variant="outlined" | ||||||
| size="small" | ||||||
| onClick={handleCopyJSON} | ||||||
| startIcon={<CheckCircleIcon sx={{ fontSize: 16 }} />} | ||||||
| startIcon={<CheckCircleIcon />} | ||||||
| > | ||||||
| Copy | ||||||
| </Button> | ||||||
|
|
||||||
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.
Ensure the Visibility icon import resolves.
AITrustCenter/index.tsxsits inpresentation/pages/AITrustCenter, so rising two levels (../../) only gets us topresentation/pages. The assets folder is atpresentation/assets, so the correct relative path is../../assets/icons/visibility-white.svg? Wait verifying. Actually.Hold on.
We already flagged path for other files. Should check this one carefully. Path currently
../../assets/icons/visibility-white.svg. Starting frompages/AITrustCenter/index.tsx:../->pages/AITrustCenter../../->pages? Wait:index.tsx-> parentAITrustCenter(1). Another up (2) ->pages. That matches:../../leads topresentation/pages. That appended withassets/icons->presentation/pages/assets/icons(non-existent). So correct path should be../../..? Let's recompute:Paths:
Clients/src/presentation/pages/AITrustCenter/index.tsx.../->Clients/src/presentation/pages/AITrustCenter.../../->Clients/src/presentation/pages.../../../->Clients/src/presentation.Hence required path is
../../../assets/icons/visibility-white.svg.Yes.
Need to adjust comment accordingly.
Let's craft.
Note: Another file resources uses
../../../? Wait they used../../../. For resources we computed../../../is correct. Good.Continue.
OK.
Next file:
AITrustCenter/Resources/index.tsximports with../../../already correct (since we computed). Need to confirm stylings for inline style.But also check
CheckCircleIconstartIcon now using ReactComponent. Withoutsxthe icon may render at default size maybe too big? Should we worry? Maybe fine.Focus on Resource file:
style={styles.visibilityIcon as React.CSSProperties}. We need to ensurestyles.visibilityIconvalues are valid CSSProperties. Without verifying we can't guarantee. Supposestyles.visibilityIconuses'fontSize: 16'or'color: "#13715B"'etc. Should we check? Possibly unstoppable. But potential error:styles.visibilityIcondefined usingSxProps. For examplestyles.visibilityIcon = { color: '#10B981', fontSize: 20 }as used in MUI, which still works when cast to CSSProperties becausefontSize: 20is allowed? In CSSProperties, numbers represent px? For certain fields accepts number. color string fine. So minimal.But there might be properties like
'backgroundColor: 'primary.main'referencing theme token. Without verifying, but it's likely actual value string.Check also they replaced Add circle import? Already existing.
Paths:
HelperDrawercomment we flagged.Need to ensure they added whitespace to import. But we flagged critical issues requiring fix.
Now ensure there are no other path errors.
Maybe there are other files not provided? The AI summary mention other files? Provided files maybe not the only ones. There might be
AITrustCenter/Resources/index.tsxusingstyles.visibilityIconwith cast. Good.Need to review
Breadcrumbschange:separator = <ChevronRightGreyIcon style={{ width: '80%', height: 'auto' }} />.Potential issue: using
width: '80%'will scale icon relative to container, but inside MUI separators (which isspan?). The icon is inline. Without view no big.But we should ensure
ChevronRightGreyIconas ReactComponent usesstylewhich expects string values;widthheightofsvgAccept. Works.Possible but the icon is
width: '80%'butheight: 'auto'maybe unnatural? Should we raise? Eh.Double-check for
VisibilityIconbeing white icon but background? In resource table they probably expect grey? Good.Focus on raising issue only for broken imports (critical). Are there more?
HelperDrawerimport path we flagged.FairnessModulepath we flagged.ResultsPagepath we flagged.AITrustCenter/index.tsxpath we flagged.AITrustCenter/Resourcespath correct.Need to ensure there aren’t other new import path requiring change like
AddCircle? Already existed.Now we must produce final review comment plus summary & blocking status? According to instructions we must produce review comments with proper format, done. Need to add comment for AITrustCenter file.
Construct comment with diff snippet.
Also ensure we include mention
Based on ???? No external.Now output for AITrustCenter.
Need to produce comment.
Also ensure we mention semicolons? Not necessary.
But also we should check general instructions—they expect final overall summary? Not specified but typical? Maybe expected to still produce general comment? Not specified but we can include final overall message summarizing issues & request changes? Usually code review includes general comment? Not mandated but we can optionally deliver general summary and overall status? Provide final note? Probably necessary.
But instructions only mention generating review comments per changed segment.
We must add comment to
AITrustCenterfile.Let's craft.
Also we should ensure comment referencing line number 8 (import line). Provide diff.
Let's produce.
Need to follow format with start and end markers.
For
AITrustCenterfile comment we need<!-- file_start ...etc, but we already have for other files? Wait we already produced for first file. We need to add new section forAITrustCenter/index.tsxcomment.Add:
Clients/src/presentation/pages/AITrustCenter/index.tsx around line 8: the
Visibility icon import uses ../../assets/... which resolves to
presentation/pages/assets (non-existent); update the import to
../../../assets/icons/visibility-white.svg so it points to
presentation/assets/icons/visibility-white.svg and ensure the file exists and
builds.