[GH-730] Add link preview for PR and issue #932
Conversation
It had use flex-related property without flex. so I added `display: flex`
…ub into link_preview
There was a problem hiding this comment.
Pull Request Overview
Adds GitHub link preview functionality for PR and issue links in chat. This allows users to see rich previews with status, title, description, labels, and other metadata when GitHub links are shared, including support for private repositories when OAuth is connected.
- Converts existing JavaScript files to TypeScript with enhanced type definitions
- Implements new LinkEmbedPreview component for rich GitHub link previews
- Updates link validation logic and styling for better label display
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/src/utils/styles.js | Updates copyright year from 2015 to 2018 |
| webapp/src/utils/github_utils.ts | Adds URL validation function for GitHub preview eligibility |
| webapp/src/types/github_types.ts | Expands type definitions for GitHub issues and pull requests |
| webapp/src/index.js | Registers new link embed preview component |
| webapp/src/components/link_tooltip/tooltip.css | Adds flex display to labels container |
| webapp/src/components/link_tooltip/link_tooltip.jsx | Updates URL validation to use new utility function |
| webapp/src/components/link_embed_preview/link_embed_preview.tsx | New component for rendering GitHub link previews |
| webapp/src/components/link_embed_preview/index.ts | Redux connector for link embed preview component |
| webapp/src/components/link_embed_preview/embed_preview.scss | Styling for link preview component |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| base: type === 'pull' && issueOrPR.base && issueOrPR.base, | ||
| head: type === 'pull' && issueOrPR.head && issueOrPR.head, |
There was a problem hiding this comment.
The conditional assignment issueOrPR.base && issueOrPR.base is redundant. The second issueOrPR.base will always be truthy if the first condition passes. Should be type === 'pull' ? issueOrPR.base : undefined.
| base: type === 'pull' && issueOrPR.base && issueOrPR.base, | |
| head: type === 'pull' && issueOrPR.head && issueOrPR.head, | |
| base: type === 'pull' ? issueOrPR.base : undefined, | |
| head: type === 'pull' ? issueOrPR.head : undefined, |
| } | ||
|
|
||
| const iconProps = { | ||
| size: 16, // Use a number instead of 'small' |
There was a problem hiding this comment.
The comment suggests this was changed from 'small' to 16, but the context shows the original code used 'small'. This inconsistency suggests the icon size prop may have changed. Verify that size: 16 is the correct replacement for 'small' in the current version of @primer/octicons-react.
| // Use a more direct approach with type assertion | ||
| // @ts-ignore - Ignoring type errors for connect function | ||
| export default connect(mapStateToProps)(LinkEmbedPreview); |
There was a problem hiding this comment.
Using @ts-ignore suppresses TypeScript errors without addressing the underlying type issues. Consider properly typing the connect function with ConnectedProps from react-redux or using the newer useSelector hook pattern instead.
| LinkEmbedPreview.propTypes = { | ||
| embed: { | ||
| url: PropTypes.string.isRequired, | ||
| }, | ||
| connected: PropTypes.bool.isRequired, | ||
| }; |
There was a problem hiding this comment.
PropTypes are redundant in TypeScript components since type checking is handled at compile time. The PropTypes definition should be removed as it's unnecessary and can lead to maintenance overhead.
wiggin77
left a comment
wiggin77
left a comment
There was a problem hiding this comment.
Some Copilot comments to review.
|
Some minor review comments to be fixed else the PR is good to go |
|
This PR has been automatically labelled "stale" because it hasn't had recent activity. |
Note: This PR contains all the changes from #779, we just updated the JS files to TS
Summary:
Make GitHub link preview for PR and issue. It can show private repository if OAuth connected.
Add display: flex on labels at link_tooltip
Ticket Link:
Fixes #730