refactor: social media URL handling improvement #165
Conversation
SafeDep Report Summary
No dependency changes detected. Nothing to scan. This report is generated by SafeDep Github App |
|
@ChrAlpha is attempting to deploy a commit to the innei-dev Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors social media URL handling to properly support both full URLs and usernames, as described in the configuration. Previously, all input was treated as usernames and directly concatenated with base URLs. The refactoring introduces a resolveSocialUrl helper function that detects full URLs (starting with http:// or https://) and returns them as-is, while constructing URLs from usernames when needed.
Key changes:
- Added
resolveSocialUrlutility function to handle URL detection and username-to-URL conversion - Pre-computed
githubUrlandtwitterUrlat component level rather than inline in JSX - Removed inline string concatenation logic from the render method
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (!normalized) { | ||
| return undefined | ||
| } |
There was a problem hiding this comment.
The check if (!normalized) on line 26 is redundant and will never be true. The replace(/^@/, '') operation only removes a leading @ character, so if trimmed is non-empty (already checked on line 17), normalized cannot be an empty string. Consider removing lines 26-28.
| if (!normalized) { | |
| return undefined | |
| } |
There was a problem hiding this comment.
ur wrong.
When trimmed is '@@' or sth like that, normalized'd be empty string.
| function resolveSocialUrl( | ||
| value: string, | ||
| { baseUrl, stripAt }: { baseUrl: string; stripAt?: boolean }, | ||
| ): string | undefined { |
There was a problem hiding this comment.
The resolveSocialUrl function lacks documentation. Add a JSDoc comment explaining its purpose, parameters, and return value. This will help future maintainers understand the URL resolution logic, especially the difference between full URLs and usernames.
|
|
||
| import { ActionGroup } from './ActionGroup' | ||
|
|
||
| function resolveSocialUrl( |
There was a problem hiding this comment.
[nitpick] Consider extracting the resolveSocialUrl function to a shared utility file (e.g., ~/lib/url-utils.ts) since this URL resolution logic could be reusable across other components that handle social media links. This would improve maintainability and testability.
2 participants
设置描述 称「支持完整链接或用户名。」
但实际上并没有做单独处理，而是直接全部视为用户名。
这个 PR 进行了重新处理。