Feature/iw 23 search engine for song search implementation#6
Feature/iw 23 search engine for song search implementation#6
Conversation
ariemic
left a comment
ariemic
left a comment
There was a problem hiding this comment.
Everthing works just fine.
There was a problem hiding this comment.
I took the liberty of getting a peek at the code. It's good in most parts. Pull changes from develop and apply Figma design. Some conventions:
- for files and directories use
kebab-case - for component names use
PascalCase - when defining widely used types/interfaces, place those into dedicated
typesdirectory
Also, remove any logs. If fetching produces any error the user should be notified.
Please fix the API key placement!
| export class SearchedVideo { | ||
| id: string; | ||
| title: string; | ||
| description: string; | ||
| thumbnailUrl: string; | ||
| channelTitle: string; | ||
| publishedAt: string; | ||
|
|
||
| constructor( | ||
| id: string, | ||
| title: string, | ||
| description: string, | ||
| thumbnailUrl: string, | ||
| channelTitle: string, | ||
| publishedAt: string, | ||
| ) { | ||
| this.id = id; | ||
| this.title = title; | ||
| this.description = description; | ||
| this.thumbnailUrl = thumbnailUrl; | ||
| this.channelTitle = channelTitle; | ||
| this.publishedAt = publishedAt; | ||
| } | ||
| } |
There was a problem hiding this comment.
This may well be a plain js object with proper typing. I guess it's some java influence there 😄
| items: YouTubeSearchItem[]; | ||
| } | ||
|
|
||
| export interface YouTubeSearchItem { |
frontend/app/(tabs)/search.tsx
Outdated
| setVideos([]); | ||
| setLoading(true); | ||
| try { | ||
| const apiKey = 'AIzaSyAcyaA3htasw-LLJQwBJvZzCbM2o6stE4s'; |
There was a problem hiding this comment.
I don't think it's safe to do sth like this...
frontend/app/(tabs)/search.tsx
Outdated
| fetchData(); | ||
| }, [debouncedSearch]); | ||
|
|
||
| const styles = StyleSheet.create({ |
There was a problem hiding this comment.
You could move these to the bottom of the file as they are not the most important structure when glancing components.
| import { SearchedVideo } from './searchedVideo' | ||
| import { YouTubeSearchItem } from './youtubeSearchResponse' | ||
|
|
||
| export const mapToSearchedVideo = (data: YouTubeSearchItem): SearchedVideo[] => { | ||
| return data.items.map( | ||
| (item) => | ||
| new SearchedVideo( | ||
| item.id.videoId, | ||
| item.snippet.title, | ||
| item.snippet.description, | ||
| item.snippet.thumbnails.medium.url, | ||
| item.snippet.channelTitle, | ||
| item.snippet.publishedAt | ||
| ) | ||
| ) | ||
| } No newline at end of file |
There was a problem hiding this comment.
I'm not sure if this is necessary at all. I'm certain there is a cleaner way to do this. Extracting just this part of the logic is not really good IMO. Also, this should be in utils directory if anything.
There was a problem hiding this comment.
This file is way too big. Segment the code into smaller pieces.
- not every function/variable needs to be declared inside the component's body as it affects performance
- declare styles at the end of the file
- is
colorsbasically thecolors.tsfile? You can import colors from there as those reflect figma's color pallete (if something is missing please add it to the file), it is located inconstantsdirectory - you can check
typography.tstoo - remove
apiKeyfrom commit history
3 participants
Search Engine is now working. Currently (because our app's layouts are not coded yet) I added the search layout into default welcome page. Run the app and at the bottom of the screen tap search button to move to search engine tab