# RoomsToGo React Style Guide

*A mostly reasonable approach to achieve consitentcy and maintenance speed in React*

## Table of Contents

1. [Component Folder Structure](#component-folder-structure)
1. [Basics](#basics)
1. [Passing Props or Push Props Up](#passing-props-or-push-props-up)
1. [Imports](#imports)
1. [Constants](#constants)
1. [Styling](#styling)

## Component Folder Structure

- use dot notation with all lowercase letters for all folders and files
> Why? It's very clear that is the same casing. No one will ever spend time guessing how to name these: folders & files like js, images, css, json, tests, stories, and others

    // bad
    / App.css
    |-- / Images
    |-- / components
    |--|- / ProductDetailsView.js
    |--|- / productDetailsImage.jpeg
    |--|- / RTGButton.js
    |--|- / RtgButton.tests.js
    |--|- / sale-flag.js
    |--|- / sale-flag.stories.js

    // good
    / app.css
    |-- / images
    |-- / components
    |--|--|-- / product.details
    |--|--|--|-- / product.details.view.js
    |--|--|--|-- / product.details.image.jpeg
    |--|--|-- / rtg.button
    |--|--|--|-- / rtg.button.js
    |--|--|--|-- / rtg.button.tests.js
    |--|--|-- / sale.flag
    |--|--|--|-- / sale.flag.js
    |--|--|--|-- / sale.flag.stories.js

- create a folder for each component which will contain the component and its helper files like styles, constants, and others.
> Why? components tend to grow so we want to be able to extract styles, constants and others into separate files for faster refactor later. Big files are harder to refactor because there is too much code noise, and you have to scroll up & down the editor.

    // bad
    |-- / components
    |--|-- / swiper.js

    // good
    |-- / components
    |--|-- / swiper
    |--|--|-- / index.js
    |--|--|-- / swiper.js
    |--|--|-- / swiper.styles.js
    |--|--|-- / swiper.constants.js // other files can be added as well

Components can be **compound** like dialog, tabs, etc. Compound components consist of many smaller related components and it's fine to place them within one file or same component folder

    // good
    |-- / dialog
    |--|- / index.js
    |--|- / dialog.js // may have DialogTitle, DialogFooter
    |--|- / dialog.styles.js
    |--|- / dialog.content.js // exports DialogContent
    |--|- / dialog.constants.js // other files can be added as 

Components can be **derived** like button and apple-button, Dialog and AlertDialog. Derived component is specialization of basic component with extra logic/styles/behavior and it's important to keep derived components separated from main implementation.

    // bad
    |-- / button
    |--|- / index.js
    |--|- / button.js
    |--|- / apple.button.js

    // good
    |-- / button
    |--|- / index.js
    |--|- / button.js
    |-- / button
    |--|- / apple.button.js

## Basics

- name the component the same as the file name

    // bad
    // inside swiper.js
    function SimpleSwiper(props) {
      // ...
    }

    // good
    // inside swiper.js
    function Swiper(props) {
      // ...
    }

- include `index.js` inside component folder, which should only re-export and do nothing else
> Why? allows encapsulation, shorter imports

- always use propTypes

## Passing Props or Push Props Up

- make sure props being passed to a component should be first and the props being pushed up should be last
- make sure props pushed up have a prefix of on-* so it’s known what that prop is doing

        // bad
        <ExampleComponent 
          passedData={data.this}
          handleFocus={onFocusEffect} 
          onBlur={onBlurEffect}
          passedDataAgain={data.here} 
        />


        // good
        <ExampleComponent 
          passedData={data.this}
          passedDataAgain={data.here} 
          onFocus={onFocusEffect} 
          onBlur={onBlurEffect}
        />

- when you are actually calling the prop function you don’t need to add the prefix

        // good
        <ExampleComponent 
          passedData={data.this}
          passedDataAgain={data.here} 
          onFocus={focusValidation} 
          onBlur={handleBlur}
        />

## Imports

- organize imports in this order
**import node modules**: display node modules first
**import common code**: display common code second
**import component code**: display component code third

    // bad
    // inside select.room.tile.js
    import THEME from '../theme
    import React from 'react'
    import EMPTY_ROOM_TEXT from './select.room.tile.constants'
    import PropTypes from 'prop-types'
    import Card from '@material-ui/core/Card'

    function SelectRoomTile(props) {
      // ...
    }

    // good
    // inside select.room.tile.js
    import React from 'react'
    import PropTypes from 'prop-types'
    import Card from '@material-ui/core/Card'
    
    import THEME from '../theme'
    import Button from '../button'
    
    import EMPTY_ROOM_TEXT from './select.room.tile.constants'
        
    function SelectRoomTile(props) {
      // ...
    }

## Constants

- SNAKE_CASE all constants
- place constant string/number/objects values inside a constant or state file instead of inside the component
Perfect candidates for these are static data for stories and tests. Other candidates are static skus, messages, min/max allowed numbers

> Why? allows to update value in a single place, allows to re-use constant in other components, tests, stories, and others

## Hook rules and patterns
- Always try to idenity reuseable hook primitives with meaningful name and single purpose.
For instance: useIsHovered, useMousePosition, useBattery
- Do not put a lot of hook code within component. If you have a lot of custom logic create separate hook named useComponentData(args)
- Keep reuseable hook within src/hooks folder at root level
- Avoid passing refs to components and subscribing to events. For isntance if you need to monitor if component is focused it's much easier to implement this hook with props getter pattern rather with passing refs.

  // bad
   function useHovered(ref) {
     const [state, setState] = useState()
       useEffect(()=>{
           // and same for mouseout
           function onSubscribe(event) {
               setState(v=>true);
           } 
           ref.current.addEventListener("mouseover", onSubscribe);
           return ()=>{
              ref.current.removeEventListener(onSubscribe);
           }
       }, [])
     return {isHovered}
   
   }

  // good
   function useHovered() {
     const [state, setState] = useState()
     const onMouseover = useCallback((event)=>{
         setState(true)
     },[])
     const onMouseout = useCallback((event)=>{
         setState(false)
     },[])
     return {isHovered:state, getProps: ()=>({ onMouseover, onMouseout })}
   }
   
   // within component
   function Button() {
       const {isHovered, getProps} = useHovered()
       return <button {...getProps()} />
   }

This way your component will be just thin wrapper which creates event handlers for component. This way you don't need to forwardRefs, you are in control of execution order and implementation is much simplier.
This is quite popular pattern in libraries like react-table and downshift.

## Testing rules and patterns
- Create testing actions which help you to write tests on more higher level of abstraction.

  
  // as you can see this way tests became much more self expanantory
  // at the same time it's easy to modify them.
  test("datepicker", async ()=>{
    // all dom nodes variables have $ sign at begining. 
    // query datepicker input on page
    const $datepickerInput = screen.getByTestId("datepicker");
    // open datepicker popup
    await openDatepickerModal($datepickerInput);
    // get reference to datepicker
    const $datepicker = await getDatepicker($datepickerInput)
    // set selected days on datepicker
    await setSelectedDays($datepicker, 5)
    // get specific days on datepicker
    const days = await getSelectedDays($datepicker);
    expect(days.length).toEqual(5);
  })  

- Create render function for common logic instead of beforeEach

  function renderComponent(optionalParams) {
    const mockFn = jest.fn();
    const testApi = render(<Provider><MyComponent onClick={mockFn}/></Provider>)
    return {
        ...testApi,
        onClickMocked: mockFn
        // ...etc
    }
  }
  test("it works", async()=>{
      const {findByText} = renderComponent();
      await findByText("something")
  })

Why? This way it's much easier to create several reuseable render function for different use cases. For instance test my component for different layouts, you can do that really easily with shared render functions and it looks much more complex with before each.
- Follow guides from testing-library https://testing-library.com/docs/guiding-principles
- More info https://kentcdodds.com/blog/?q=testing

## Global Styling

- TODO: global styling: theme vs tokens (constants)

## Component Styling
- Prefer object syntax over styled tag syntax

  const StyledButtonBad = styled.div`
   color: red;
  `
  const StyledButtonGood = styled.div(()=>({
      color: "red"
  }))
  `

Why? We need to be consistent at styles definitions and object syntax is the least evil. Working with objects is much more simpler and easier to migrate from one library to another.

**[⬆ back to top](#table-of-contents)**