What needs to be worked on, what to improve?

Question

Career in IT

Hello everyone,
I'm learning frontend, I'm taking Hexlet courses, I
made my first test task from Aviasales on React :
https://marinarodkin.github.io/aviasales-app/
https://github.com/marinarodkin/aviasales-app
my level? What needs to be worked on, in what direction? I will do a few more test tasks for the git hub. The goal is to go to work as a front-end,
I myself can say that at least you need to finish responsive, convert units of measurement in CSS to relative ones,
simplify the filter logic (but need a hint how)
thanks in advance for the answers!

Answer 1

1. Commented code on github is not good. https://github.com/marinarodkin/aviasales-app/blob...
   
2. Minimum logic in the component 's render function. Transfer all complex constructions to methods, and preferably to separate components (then you can more easily control the redrawing of components through shouldComponentUpdate so that they do not redraw if the data has not changed). You can write arrow functions just like methods:
   

   class Flight extends Component {
       getWeekDay = (date) => {
           //..
       }
       // ....
   }

   
   

   const newDate  = new Date (year, month, day, );
   const monthName = ["дек", "янв", "фев", "мар", "апр", "мая", "июня", "июля", "авг", "сент", "окт", "ноя", "дек"];
   const newMonth = monthName[newDate.getMonth()];

   
   The Date constructor expects a zero-based month as its second argument. those. 0 - January, 1 - February, 11 - December. Judging by monthName , you suspected that there was something wrong, but you made a mistake with the implementation. monthName must have a normal form, starting with January and ending with December, because the zero element of the array just fits the logic with a zero month. In getDateFormat , and also in getWeekDay , subtract from month - 1
   

   const getStopsNumber = (stop) =>{
         switch (stop) {
           case 3:
             return "3 пересадки"
           case 2:
             return "2 пересадки"
           case 1:
             return "1 пересадка"
           case 0:
             return "без пересадок"
           default:
             return // это не нужно делать, писать return. Если вы удалите эту (и строку выше), то результат будет такой же - undefined
         }
       }

   
   
3. If in the SideBar the stopsData prop was not an object, but a string or a number, then the SideBar component could be painlessly turned into a PureComponent. Well it so, by the way about optimization.
   
4. I would not pass the event object e to stopsClick , from which you then take the element id through e.target.id (which is not good), but I would make an arrow function (or bind), into which I would pass the id. Like this
   

   <input onClick={() => this.props.stopsClick("allStops")} />
   <input onClick={() => this.props.stopsClick("noStops")} />

   
   If experienced ReactJS developers are reading this, please consider. I agree that each component will have its own copy of the function, but at least you don’t need to interact with the DOM directly.
   
5. It's not beautiful
   

   if( this.state.stops.allStops === false && this.state.stops.noStops === true && this.state.stops.oneStop === true && this.state.stops.twoStop === true && this.state.stops.threeStop === true  ){
            newStops = {...this.state.stops, allStops: true}
       }

   
   It seems to me that this does not play a role in further logic, it only creates a glitch when you select all checkboxes except "all", and if you click on one of them after that, it will not be released, but only the "all" checkbox will turn on.
   Maybe I sucked a lot out of my finger, but it will be useful to you. Learn, develop. Good luck with that :-)

Answer 2

I'm not a fan of react, so I won't talk about it. But I will criticize CSS:
- It is worth screwing up some preprocessor, using nesting (the structure will be better visible) and putting everything that is taken out into human-understandable constants - colors, sizes, etc. There are enough repetitions.
- It is worth dividing everything into separate component files.
- It is worth thinking more about the overall breakdown of CSS into components. Of course, there are different approaches, but individual buttons, or a group of several buttons, or checkboxes are universal things for the entire project. What's the point of linking them to some sidebar or calculator?
- You yourself wrote about adaptability.
- Styles for :focus are missing. The keyboard cannot be used.
- It also seems to me that the sidebar should have an indent at the bottom (I haven’t seen the design, but IMHO it is). And that the buttons should have cursor: pointer.
ZY: There is also an idea that the "all" option is not needed there. "All" should be shown in the absence of filters. But without an analysis of the target audience, I won’t say, maybe people are used to this option there.

Answer 3

Well, here's a side view.
1. Learn how to design projects with markdown. Now such a description of the project is hard to read, I mastered 5 words, a potential employer would not have looked at all.
2. Group project files by type and meaning. Now it’s a mess and it’s hard for me to understand as a simple non-frontender where the logic is, and where the framework is.
3. Since you are still writing shitty code, get used to writing comments over complex sections of code where the essence of the function execution is expressed.
4. I don’t know how it is in the frontend now, but backenders don’t like it when logic and patterns interfere.
5. Learn how to place brackets, read how to format code in js

spoiler

componentDidMount() {
    this.getTicketData("ticket.json");
      } // И так везде

6. What is this?
7. This is bad practice. The code should be abstracted from the data. It is necessary to replace with an object and check whether the object contains the necessary data or return a default value.

spoiler

const getStopsNumber = (stop) =>{
      switch (stop) {
        case 3:
          return "3 пересадки"
        case 2:
          return "2 пересадки"
        case 1:
          return "1 пересадка"
        case 0:
          return "без пересадок"
        default:
          return
      }
    }

Enough for now.

Answer 4

I will add. You work with React, but at the same time you just installed templates, which does not make sense, you can take absolutely any framework there and throw templates and hang handlers without a framework at all. You need to break the theme into components, into which you pass data through props, otherwise you have a lot of repetitive html
. Install a linter, it will help you. With him, you will even shit-code, but it's beautiful

Answer 5

I noticed that you have many functions defined in the render methods. This is not necessary, but rather should be avoided - either take it out to the class level if this is needed, or take it out of it.
You also go for a picture and a json file using a link to the github, or you can connect them via import, because they are locally available for the React application. If it’s still debatable with json, then the picture in the header definitely needs to be imported

Answer 6

This is generally for js code than purely for react.

switch(curr){
      case "eur":
        newCurrency = {...this.state.currencyData, rub: false, usd: false, eur: true };
        break
      case "usd":
        newCurrency = {...this.state.currencyData, rub: false, usd: true, eur: false };
        break
      default:
        newCurrency = {...this.state.currencyData, rub: true, usd: false, eur: false };
        break
      }

And if there is another currency, will you add more properties? It's better to keep the currency property and build on it. There would be typescript, they would also indicate which lines can be assigned where. Moreover, there is already currency in the state, why I did not understand currencyData.
It's the same with all stop properties, there are a lot of them, it's hard to understand what's what. In general, look at this first.