-
Notifications
You must be signed in to change notification settings - Fork 77
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
implement memorial photo viewer #119
base: v1-dev
Are you sure you want to change the base?
Conversation
will prob work on my mac ltr
文件级别变更
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以:
获得帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request implements a new page to display memorial photos uploaded from MaiMai. It adds a new route '/pictures', a new Sequence diagram for fetching memorial photossequenceDiagram
participant User
participant Browser
participant Server
User->>Browser: Clicks on "pictures" link
Browser->>Server: GET /api/v2/game/mai2/my-photo
Server-->>Browser: Returns list of photo URLs
Browser->>Browser: Renders images from URLs
Browser-->>User: Displays memorial photos
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @alixdotsh - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding a loading state or skeleton UI while fetching photos to improve user experience.
- The token is being passed as a query parameter, which might be logged by some servers; consider using a header instead.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
GAME.photos().then(result => { | ||
console.log(result) | ||
photos = result | ||
}).catch(e => { |
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.
suggestion: Consider using onMount for asynchronous operations.
Moving the GAME.photos() call into an onMount block is the recommended pattern in Svelte to ensure the fetching happens after the component has been mounted.
Suggested implementation:
import { onMount } from 'svelte';
import {GAME} from "../libs/sdk";
import {AQUA_HOST} from "../libs/config";
let photos: string[] = [];
onMount(() => {
GAME.photos()
.then(result => {
console.log(result);
photos = result;
})
.catch(e => {
console.error(e);
});
});
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.
Looks good. Although I don’t get final say on this lol
Looks good, maybe consider using Svelte's await feature? |
talked abt in disc primarily for the most of it. the css seems fine for mobile but i could always be wrong i lowkey have not done regular css in a hot minute LOL
open to feedback and suggestions :3
好的,这是翻译成中文的 pull request 总结:
Sourcery 总结
为 Mai 游戏实现纪念照片查看器,允许用户查看他们上传的照片。
新功能:
Original summary in English
Summary by Sourcery
Implements a memorial photo viewer for the Mai game, allowing users to view their uploaded photos.
New Features: