feat: add en.scansgg#531
Conversation
Olaaris
force-pushed
the
feat/en-scansgg-477
branch
from
72ec0eb to
76a4b05
Compare
Olaaris
force-pushed
the
feat/en-scansgg-477
branch
from
76a4b05 to
aba983e
Compare
Olaaris
force-pushed
the
feat/en-scansgg-477
branch
from
aba983e to
429f84c
Compare
kkantan
left a comment
kkantan
left a comment
There was a problem hiding this comment.
looks good overall, just left a few refactoring suggestions.
| { | ||
| "info": { | ||
| "id": "en.scansgg", | ||
| "name": "Scans.GG", |
There was a problem hiding this comment.
It looks like the website title uses lowercase for the gg, so that might be more accurate.
| "name": "Scans.GG", | |
| "name": "Scans.gg", |
| "name": "Scans.GG", | ||
| "version": 1, | ||
| "url": "https://scans.gg", | ||
| "contentRating": 0, |
There was a problem hiding this comment.
given that there is a "Hentai" genre and such, this should be marked as containing nsfw content.
| "contentRating": 0, | |
| "contentRating": 1, |
| #[derive(Deserialize)] | ||
| struct Response<T> { | ||
| data: T, | ||
| } |
There was a problem hiding this comment.
it would be better to put all the struct definitions in a models.rs folder, to be consistent with other sources.
| fn cover_url(cover: Option<&str>) -> Option<String> { | ||
| cover.map(|cover| format!("{CDN_URL}/covers/{cover}")) | ||
| } |
There was a problem hiding this comment.
these extra functions could go in a helpers.rs file.
| } | ||
| } | ||
|
|
||
| fn manga(&self, tag_list: Option<&[Tag]>) -> Manga { |
There was a problem hiding this comment.
can we consume self for these two functions? this would allow us to avoid cloning everything.
| fn manga(&self, tag_list: Option<&[Tag]>) -> Manga { | |
| fn manga(self, tag_list: Option<&[Tag]>) -> Manga { |
| let mut response = Request::get(format!("{API_URL}/series?{qs}"))?.send()?; | ||
| let data = response.get_json::<Response<Vec<Series>>>()?; |
There was a problem hiding this comment.
there's a shorthand function you could use here, but it does the same thing so it doesn't really matter.
| let mut response = Request::get(format!("{API_URL}/series?{qs}"))?.send()?; | |
| let data = response.get_json::<Response<Vec<Series>>>()?; | |
| let data = Request::get(format!("{API_URL}/series?{qs}"))? | |
| .json_owned::<Response<Vec<Series>>>()?; |
| let data = response.get_json::<Response<Series>>()?; | ||
| let tag_list = fetch_tags().ok(); | ||
| manga.copy_from(data.data.manga(tag_list.as_deref())); | ||
| send_partial_result(&manga); |
There was a problem hiding this comment.
usually we put this in an if branch, though I'm not sure if it really matters.
| send_partial_result(&manga); | |
| if needs_chapters { | |
| send_partial_result(&manga); | |
| } |
| push_scroller( | ||
| &mut components, | ||
| "Popular Last 3 Months", | ||
| popular.three_months, | ||
| ); | ||
| push_scroller(&mut components, "Popular Last 6 Months", popular.six_months); |
There was a problem hiding this comment.
I'm not sure how worthwhile it is having these, since they're identical to the one year scroller.
| } | ||
|
|
||
| push_scroller(&mut components, "Latest Updates", data.data.latest_updates); | ||
| push_scroller(&mut components, "Series", data.data.series); |
There was a problem hiding this comment.
this looks like the same data as the site lists under "New Series", so that might be a better title.
There was a problem hiding this comment.
is there any chance the padding could be increased a bit around the icon? it seems a little close to the edge, especially compared to the icon shown in the top left corner on the website.
2 participants
Hello ! First full source so be gentle :D
Let me know if I need to change anything, I've tested through a locally hosted source list and things look good.
Related to #477
Summary
Validation