Skip to content

Conversation

@armenzg
Copy link
Member

@armenzg armenzg commented Jun 6, 2025

This expands the criteria to show the Screenshots component when at least one attachment ends with screenshot.png or screenshot.jpg.

This is a follow-up to #91602.

This widget:
image

…he attachments

This expands the criteria to show the Screenshots component when at least one attachment ends with `screenshot.png` or `screenshot.jpg`.
@armenzg armenzg self-assigned this Jun 6, 2025
@armenzg armenzg requested a review from scttcper June 6, 2025 14:17
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Jun 6, 2025
@armenzg armenzg marked this pull request as ready for review June 6, 2025 20:21
@armenzg armenzg requested a review from a team as a code owner June 6, 2025 20:21
const screenshots =
attachments?.filter(({name}) => SCREENSHOT_NAMES.includes(name)) ?? [];
attachments?.filter(
({name}) => name.endsWith('screenshot.jpg') || name.endsWith('screenshot.png')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we know if "screenshot-1" is still a case we want to cover?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will expand the test case on Monday. There's no one with certainty as to what they want.

const screenshots =
attachments?.filter(({name}) => SCREENSHOT_NAMES.includes(name)) ?? [];
attachments?.filter(
({name}) => name.endsWith('screenshot.jpg') || name.endsWith('screenshot.png')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wont this possibly cause a regression? SCREENSHOT NAMES also includes:

'screenshot-1.jpg',
'screenshot-1.png',
'screenshot-2.jpg',
'screenshot-2.png',

which this wont pick up now right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm oops, yea same as what Richard said

@armenzg armenzg force-pushed the feat/show_screenshot/armenzg branch from a870ad4 to 6be73a6 Compare June 9, 2025 13:14
@armenzg armenzg enabled auto-merge (squash) June 9, 2025 13:14
@armenzg armenzg merged commit 3c60ee2 into master Jun 9, 2025
43 checks passed
@armenzg armenzg deleted the feat/show_screenshot/armenzg branch June 9, 2025 13:25
andrewshie-sentry pushed a commit that referenced this pull request Jun 19, 2025
This expands the criteria to show the Screenshots component when at
least one attachment ends with `screenshot.png` or `screenshot.jpg`.

This is a follow-up to #91602.

This widget:
<img width="164" alt="image"
src="https://github.com/user-attachments/assets/37bafd95-0464-4604-b0f8-46e6479ce29a"
/>
@github-actions github-actions bot locked and limited conversation to collaborators Jun 25, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants