Skip to content

Conversation

@maxeasy18
Copy link
Collaborator

работает Utube

previews: [],
searchOpts: {
maxResults: 10,
key: 'AIzaSyC1ORL6Y3zxvLLev6QHUqP8eF1hFbYo1WI',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the key is not a part of a state

activeVideo : {}
}
this.updateSearch = this.updateSearch.bind(this);
this.getPreviews = this.getPreviews.bind(this);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you don't need to bind instance methods


return (
<div className="video-detail col-md-8">
{console.log(linkToVideo)}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure you need a console.log in jsx

import React, { Component } from "react";
import {VideoPreview} from "./video_preview";

const renderPreviews = (previews,activatePreview) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a prototype method is fine, an additional function is a bit overhead

render () {
const preview = this.props.preview;
return (
<li className="list-group-item" onClick={() => this.props.activatePreview(preview)}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's better to create an instance method which would be responsible for click instead of doing it inside JSX.

You current JSX would create new function every time if components renders

<iframe title="random" src={linkToVideo}/>
</div>
<div className="details">
<div>{video.title}</div>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please extract title, description, id from props

super();
this.state = {
previews: [],
searchOpts: {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

searchOpts is not a part of a state, they could be static and refers only to API

searchOpts: {
maxResults: 10,
key: 'AIzaSyC1ORL6Y3zxvLLev6QHUqP8eF1hFbYo1WI',
videoEmbeddable: true,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't found any place where you use videoEmbeddable, is it required ?

key: 'AIzaSyC1ORL6Y3zxvLLev6QHUqP8eF1hFbYo1WI',
videoEmbeddable: true,

type:"video"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as above

}

getPreviews = () => {
Search(this.state.searchText, this.state.searchOpts, (err, results) => {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would split request and request handlers to different methods, but it's okey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants