-
Notifications
You must be signed in to change notification settings - Fork 0
Add app icon display to Preprod Build Details sidebar #92
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
base: ryan/add_api_to_serve_preprod_app_icons
Are you sure you want to change the base?
Add app icon display to Preprod Build Details sidebar #92
Conversation
…app_icons' into ryan/add_api_to_serve_preprod_app_icons
|
Display uploaded app icon in build-details sidebar Adds front-end support for rendering the specific app icon, if it exists, in the pre-production build details sidebar. The component now derives an icon URL using the Key Changes• Added Affected Areas• This summary was automatically generated by @propel-code-bot |
| {iconUrl && <img src={iconUrl} alt="App Icon" width={24} height={24} />} | ||
| {!iconUrl && ( | ||
| <AppIconPlaceholder>{props.appInfo.name?.charAt(0) || ''}</AppIconPlaceholder> | ||
| </AppIcon> | ||
| )} |
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.
[BestPractice]
Missing error handling for image loading: If the constructed iconUrl is invalid or the image fails to load, users will see a broken image. Add error handling to fallback to the placeholder:
const [imageError, setImageError] = useState(false);
// In the JSX:
{iconUrl && !imageError && (
<img
src={iconUrl}
alt="App Icon"
width={24}
height={24}
onError={() => setImageError(true)}
/>
)}
{(!iconUrl || imageError) && (
<AppIconPlaceholder>{props.appInfo.name?.charAt(0) || ''}</AppIconPlaceholder>
)}Note: You'll need to add import {useState} from 'react'; at the top of the file.
Context for Agents
[**BestPractice**]
Missing error handling for image loading: If the constructed `iconUrl` is invalid or the image fails to load, users will see a broken image. Add error handling to fallback to the placeholder:
```typescript
const [imageError, setImageError] = useState(false);
// In the JSX:
{iconUrl && !imageError && (
<img
src={iconUrl}
alt="App Icon"
width={24}
height={24}
onError={() => setImageError(true)}
/>
)}
{(!iconUrl || imageError) && (
<AppIconPlaceholder>{props.appInfo.name?.charAt(0) || ''}</AppIconPlaceholder>
)}
```
Note: You'll need to add `import {useState} from 'react';` at the top of the file.
File: static/app/views/preprod/buildDetails/sidebar/buildDetailsSidebarAppInfo.tsx
Line: 88| <Flex direction="column" gap="xl"> | ||
| <Flex align="center" gap="sm"> | ||
| <AppIcon> | ||
| {iconUrl && <img src={iconUrl} alt="App Icon" width={24} height={24} />} |
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.
[BestPractice]
Missing accessibility attributes: The <img> element should include proper accessibility attributes for better screen reader support:
| {iconUrl && <img src={iconUrl} alt="App Icon" width={24} height={24} />} | |
| {iconUrl && <img src={iconUrl} alt={props.appInfo.name ? `${props.appInfo.name} app icon` : 'App icon'} width={24} height={24} />} |
⚡ Committable suggestion
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
Context for Agents
[**BestPractice**]
Missing accessibility attributes: The `<img>` element should include proper accessibility attributes for better screen reader support:
```suggestion
{iconUrl && <img src={iconUrl} alt={props.appInfo.name ? `${props.appInfo.name} app icon` : 'App icon'} width={24} height={24} />}
```
⚡ **Committable suggestion**
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
File: static/app/views/preprod/buildDetails/sidebar/buildDetailsSidebarAppInfo.tsx
Line: 85dec7590 to
1027469
Compare
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Copied from getsentry#102118
Original PR: getsentry#102118