Smells in React Apps [with edited transcript]

Smells in React Apps [with edited transcript]

Presented at JSConf.Asia 2018. This one comes with edited transcript.

47fd6e7bd2b7bf8f2e3f2a51c7ffa53d?s=128

Thai Pangsakulyanont

January 27, 2018
Tweet

Transcript

  1. Thai Pangsakulyanont 1 Smells in React Apps JSConf.Asia 2018 (with

    transcript added) Note: The transcript is extracted and edited from the recorded talk.
  2. Slide № 2 This is a blank slide.

  3. Slide № 3 Sawasdee krub (Hello in Thai) This is

    my first time giving a full-length talk at a conference outside Thailand. I’m very grateful to the organizers for having invited me here.
  4. Slide № 4 Smells in React Apps Today, I will

    talk about
  5. Slide № 5 Refactoring: Improving the Design of Existing Code

    Clean Code: A Handbook of Agile Software Craftsmanship Agile Software Development, Principles, Patterns, and Practices There are many books that talk about code smell: Unfortunately, I don’t read many books. Usually, I read articles on the internet and piece the information together. So, these 3 books, I haven’t read them yet.
  6. Slide № 6 (Contents afterwards are mainly based on my

    experience.) They may not perfectly align with those books. I may have used a different term. I really hope they will be useful to you. Therefore…
  7. About Me Slide № 7 Thai Pangsakulyanont (@dtinth) Please tweet

    me your feedback. (from Thailand)
  8. Slide № 8 Find me on GitHub:

  9. Slide № 9 Loves to build software for fun. In

    my free time, I like to work on side projects.
  10. Slide № 10 A web-based musical instrument with several instruments

    to choose from. I demoed it at JSConf.Asia 2016, in a lightning talk. dtinth/midi-instruments https://302.dt.in.th/midi-instruments-pv
  11. Slide № 11 dtinth/atom-aesthetic-ui Some other time, I feel like

    doing something a bit silly. So, I created a Windows 2000 theme for Atom text editor.
  12. Slide № 12 dtinth/atom-aesthetic-ui Main challenge: Replicate the Windows Explorer

    tree view using CSS. That was really fun!
  13. Slide № 13 dtinth/GyroscratchAndroid bemusic/gyroscratch-web The other time, I want

    to play a music game, like a DJ. I need a turntable. So I put my phone on a dish, and make it transmit gyroscope data to the game. https://302.dt.in.th/gyroscratch-pv
  14. Slide № 14 Loves to build software for fun. For

    me the “fun” part is very important. It makes coding a fulfilling thing to do. It can also improve my professional coding skills. So there you see it.
  15. Slide № 15 Interested in the ways
 to make software

    maintainable. I am also
  16. Slide № 16 One of my old project… (2009) That’s

    because… I put in a lot of effort developing it. At that time, I didn’t learn much about software engineering, so I keep on coding and coding. Over time, the project become less maintainable.
  17. Slide № 17 7000 lines of untestable code. There we

    have… At that point, when I do something or fix something, more bugs appear.
  18. Slide № 18 7000 lines of untestable code. Maintain: Not

    fun Later, I learn about software engineering. I feel that I want to improve the codebase, add tests, etc.
  19. Slide № 19 7000 lines of untestable code. Maintain: Not

    fun Improve the codebase: Not fun Because I have… It is a huge boring task! A side project should be fun, not a burden. I start to feel that I’d rather do something else more fun.
  20. Slide № 20 Development stopped So…

  21. Slide № 21 Development stopped :( That’s sad, because I

    spent years working it.
  22. Slide № 22 What I learned…

  23. Slide № 23 Sustainability matters! When I work on a

    new project, I need to make sure it will not end up like that project again.
  24. Slide № 24 Interested in the ways
 to make software

    maintainable. That’s why I’m interested in these stuff:
  25. Slide № 25 Object-oriented analysis and design Clean architecture Functional

    programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … There are many contributing factors. Many techniques you can use to make software more maintainable.
  26. Slide № 26 Later… (2014)

  27. Slide № 27 bemusic/bemuse bemusic/bemuse https://302.dt.in.th/bemuse-pv ADVERTISEMENT https://bemuse.ninja/ I started

    working on a new project, called Bemuse. It’s a web-based, open-source music game built using HTML5 technologies. I want to make it a great game and I want people to play it. So I need to make sure this project is maintainable.
  28. Slide № 28 I organize my code into modules…

  29. Slide № 29 …and use to bundle them together.

  30. Slide № 30 Static analysis with ESLint I set up

    ESLint to catch common errors…
  31. Slide № 31 Static analysis with ESLint …and enforce a

    consistent coding style.
  32. Slide № 32 Unit testing (Mocha)

  33. Slide № 33 Continuous integration & deployment (CircleCI)

  34. Slide № 34 Code coverage (Codecov)

  35. Slide № 35 3 weeks of project setup It took

    me No game development during that time. Sounds pretty long compared to other projects.
  36. Slide № 36 Result It made a huge impact.

  37. Slide № 37 :) Unit tests increases confidence when making

    a huge change to the codebase. They helped me confidently switch to use another library, without the fear that I might break the game
  38. Slide № 38 Decoupled architecture It allowed me to try

    out different view libraries (2 times) without having to rewrite the whole app.
  39. Slide № 39 Implementation details behind an interface It allowed

    me to switch to a new backend service without having to make a sweeping change across the whole codebase
  40. Slide № 40 Some features can be prototyped in 1

    night
  41. Slide № 41 A good codebase is like a clean

    table. I learned that… In a good codebase, I can experiment quickly and prototype new ideas right away…
  42. Slide № 42 …without having to first move so many

    stuffs that’s getting in my way! After the experiment is finished…
  43. Slide № 43 …I try to refactor the codebase to

    be clean again, …ready to take in more experiments to come in the future.
  44. Slide № 44 I’ve been maintaining this game for 3

    years, and I’m still maintaining it to this day.
  45. Slide № 45 Present (2018)

  46. Slide № 46 Front-end architect at @taskworld * Opinion expressed

    are my own.
  47. Slide № 47 React + Redux Our app is built

    using
  48. Slide № 48 We have many features. This is the

    tree view of our Redux store.
  49. Slide № 49 Redux store

  50. Slide № 50 Redux store React components

  51. Slide № 51 Redux store React components 900+ components

  52. Slide № 52 Redux store React components 900+ components Over

    2 years
  53. Slide № 53 Redux store React components 900+ components Over

    2 years, 20+ engineers worked on it It is a challenge for us to keep innovating while making sure the code is maintainable. Have to strike a healthy balance: minimizing tech debt vs shortening time to market
  54. Slide № 54 Object-oriented analysis and design Clean architecture Functional

    programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … As I said, there are many contributing factors:
  55. Slide № 55 Object-oriented analysis and design Clean architecture Functional

    programming Automated testing Continuous integration Linting Type systems Modularization Componentization Code smells … Tech stuffs The things that I listed here are…
  56. Slide № 56 Communication But when working in a team,

    I learned that the most important thing really is
  57. Slide № 57 Example

  58. Slide № 58 // MusicPreviewer.js export function preload () {

    getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } MusicPreviewer
  59. Slide № 59 // MusicPreviewer.js export function preload () {

    getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } MusicPreviewer exports 5 functions
  60. Slide № 60 // MusicPreviewer.js export function preload () {

    getInstance() } export function enable () { return getInstance().enable() } export function disable () { return getInstance().disable() } export function go () { return getInstance().go() } export function preview (url) { return getInstance().preview(url) } import * as MusicPreviewer from MusicPreviewer.preload() MusicPreviewer.enable() MusicPreviewer.disable() MusicPreviewer.go() MusicPreviewer.preview(url) User code: The rest of the app only needs to know about these 5 functions
  61. Slide № 61 Behind the interface They don’t need to

    know that… …lies a big, ugly hack.
  62. Slide № 62 let instance const getInstance = () =>

    instance || ( instance = createMusicPreviewer() ) function createMusicPreviewer () { let enabled = false let currentUrl = null let backgroundLoaded = false let backgroundPlayed = false const instances = {} function update () { if (!enabled) { if (backgroundPlayed) { backgroundFader.fadeTo(0, 100) backgroundPlayed = false background.pause() } for (const key of Object.keys(instances)) { const instance = instances[key] instance.destroy() } return } let playing = null for (const key of Object.keys(instances)) { const instance = instances[key] if (key === currentUrl) { if (instance.loaded) { instance.play() playing = instance } else { instance.stop() } } if (playing) { backgroundFader.fadeTo(0, 1) } else { backgroundFader.fadeTo(0.4, 0.5) if (backgroundLoaded && !backgroundPlayed) { backgroundPlayed = true try { background.play() } catch (e) { console.warn('Cannot play background music') } } } } const musicPreviewer = { enable () { if (enabled) return enabled = true update() }, disable () { if (!enabled) return enabled = false update() }, go () { if (!enabled) return goSound.currentTime = 0
  63. Slide № 63 Wrote a hack behind a well-designed interface

    In principle: Bad code stays behind that interface. This would work fine. Other parts of software not affected by this.
  64. Slide № 64 Wrote a hack behind a well-designed interface

    In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution,
  65. Slide № 65 Wrote a hack behind a well-designed interface

    In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution, found my hack (but didn’t know it was a hack), because I didn’t communicate clearly
  66. Slide № 66 Wrote a hack behind a well-designed interface

    In principle: Bad code stays behind that interface. In practice: Another person looks for a technical solution, found my hack (but didn’t know it was a hack), and duplicated the code! If it’s a hack/non-exemplar code, then it’s really important to communicate why the code was written that way.
  67. Slide № 67 Effective code reviews Another important factor is

    having…
  68. Slide № 68 Smells To be effective, in my opinion,

    we should learn to recognize the… …inside our code.
  69. Slide № 69 Information leaks The first kind of smell,

    I call it:
  70. Slide № 70 Example

  71. Slide № 71 Standard UI kit In our project, we

    have: …which contains the common components used across our app
  72. Slide № 72 A UserAvatar component Can be used to

    render user’s avatar by specifying the image and size.
  73. Slide № 73 <UserAvatar size='small' … /> A UserAvatar component

  74. Slide № 74 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> A UserAvatar component
  75. Slide № 75 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component (Oops, that’s a bit too large…)
  76. Slide № 76 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component
  77. Slide № 77 New feature! Messaging system The code changes

    over time. Later, as we develop, we need to implement new features.
  78. Slide № 78 Thai Pangsakulyanont 1 minute ago Let’s go

    to the Marina Bay Carnival after the conference!!!! UserAvatar on the left, text on the right… This should be straightforward. This is how a message should look like.
  79. Slide № 79 So, I look at the UserAvatar component…

  80. Slide № 80 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component
  81. Slide № 81 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component the sized that we need to use is not there!
  82. Slide № 82 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component To fix this, we need to add a new size. But we already have small, medium, and large. What should we put in here?
  83. Slide № 83 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component <UserAvatar size='message' … /> We did the most obvious thing: Does this smell a bit to you? Let’s keep going on!
  84. Slide № 84 Few months later

  85. Slide № 85 Few months later everywhere! <UserAvatar size='message' …

    /> …in places totally unrelated to the message
  86. Slide № 86 Step out of code Look at the

    big picture
  87. Slide № 87 UserAvatar Message

  88. Slide № 88 UserAvatar specific feature Message

  89. Slide № 89 UserAvatar specific feature generic component Message

  90. Slide № 90 UserAvatar specific feature generic component I need

    to show an avatar! Message
  91. Slide № 91 UserAvatar uses specific feature generic component Message

    I need to show an avatar!
  92. Slide № 92 UserAvatar uses specific feature generic component I

    know how big to display in a message Message
  93. Slide № 93 UserAvatar uses aware of specific feature generic

    component Message I know how big to display in a message
  94. Slide № 94 UserAvatar uses aware of specific feature generic

    component Information leaking. Message
  95. Slide № 95 UserAvatar Message uses aware of specific feature

    generic component Information leaking. Knows how to render itself Except: Doesn’t know the appropriate size of the avatar
  96. Slide № 96 UserAvatar Message uses aware of specific feature

    generic component Information leaking. Knows how to render itself Except: Doesn’t know the appropriate size of the avatar The knowledge of the size, which should have belonged to the messaging system is here That knowledge leaked into the UserAvatar component.
  97. Slide № 97 Leakage Information about a specific feature leaked

    into a generic component …making the component less generic, more awkward to reuse
  98. Slide № 98 Generic component not flexible enough. Why did

    this happen? Maybe…
  99. Slide № 99 Make the generic component more flexible Fix:

  100. Slide № 100 <UserAvatar size='small' … /> <UserAvatar size='medium' …

    /> <UserAvatar size='large' … /> A UserAvatar component <UserAvatar size='message' … /> We used to have 4 sizes:
  101. Slide № 101 A UserAvatar component XS S M L

    XL XXL XXXL Now we have 8 sizes:
  102. Slide № 102 UserAvatar specific feature generic component I need

    to show an avatar! It’s going to have size M! Message Now Message can pick a size from the catalog.
  103. Slide № 103 A UserAvatar component function UserAvatar (props) {

    … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 'XS', 'S', 'M', 'L', 'XL', 'XXL', 'XX … } Pitfall: What would happen if we want to make a new size between L and XL? We may need to adjust the size table and that means a sweeping change across the codebase to shift sizes. Fortunately this is flexible enough at Taskworld.
  104. Slide № 104 A UserAvatar component function UserAvatar (props) {

    … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 'XS', 'S', 'M', 'L', 'XL', 'XXL', 'XX … } If you want to make it really flexible…
  105. Slide № 105 A UserAvatar component function UserAvatar (props) {

    … } UserAvatar.propTypes = { size: PropTypes.number, … } Let the users specify the size in pixel units. But it may go out of control.
  106. Slide № 106 A UserAvatar component function UserAvatar (props) {

    … } UserAvatar.propTypes = { size: PropTypes.oneOf([ 18, 24, 30, 36, 48, … ]), … } To keep things under control, you have a list of allowed sizes. There are trade-offs to all these approaches. This is an example where a smell calls for a bit of restructuring…
  107. Slide № 107 Sometimes it’s just naming. But… All you

    have to do is just changing the name!
  108. Slide № 108 Example

  109. Slide № 109 Standard UI kit

  110. Slide № 110 Upload file

  111. Slide № 111 Upload file The user can click on

    a button.
  112. Slide № 112 Upload file

  113. Slide № 113 Upload file Select file on computer… Dropbox

    Google Drive Emergency alert
  114. Slide № 114 Upload file Select file on computer… Dropbox

    Google Drive Emergency alert Select file on computer… The user can select an item from a menu.
  115. Slide № 115 Upload file Select file on computer… Dropbox

    Google Drive Emergency alert <Button> I have a <Button>
  116. Slide № 116 Upload file Select file on computer… Dropbox

    Google Drive Emergency alert <Menu> <Button> I have a <Menu>
  117. Slide № 117 Select file on computer… Dropbox Google Drive

    Emergency alert Upload file Ah!!!
  118. Slide № 118 Select file on computer… Dropbox Google Drive

    Emergency alert <MenuButton> Upload file <MenuButton>!
  119. Slide № 119 Upload file <MenuButton>

  120. Slide № 120 <MenuButton> Upload file When you click on

    a MenuButton…
  121. Slide № 121 Select file on computer… Dropbox Google Drive

    Emergency alert <MenuButton> Upload file …a menu opens…
  122. Slide № 122 Select file on computer… Dropbox Google Drive

    Emergency alert <MenuButton> Upload file Select file on computer… …and you can select items from it.
  123. Slide № 123 Select file on computer… Dropbox Google Drive

    Emergency alert <MenuButton> Upload file Select file on computer… Button must appear to be pressed while menu is open The catch:
  124. Slide № 124 Upload file <MenuButton> Only when the menu

    is closed, then the button can return to its normal state.
  125. Slide № 125 Upload file Button.propTypes = { children: PropTypes.node

    } At first thought, we may start with the most obvious thing here…
  126. Slide № 126 Upload file Button.propTypes = { children: PropTypes.node,

    isMenuOpen: PropTypes.bool }
  127. Slide № 127 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool

    } Upload file If true, then the button appears to be pressed. Again… Does it smell a bit?
  128. Slide № 128 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool

    } Button is now aware of the menu Upload file An entirely unrelated concept!
  129. Slide № 129 Button.propTypes = { children: PropTypes.node, isMenuOpen: PropTypes.bool

    } Upload file Thinking a bit deeper, we see that a <Button> doesn’t really need to know about the menu.
  130. Slide № 130 Button.propTypes = { children: PropTypes.node, isDepressed: PropTypes.bool

    } Upload file It only needs to know whether it should appear to be…depressed. A simple naming change can entirely communicate a different thing!
  131. Slide № 131 When reviewing code: “Does this really needs

    to know about that?”
  132. Slide № 132 Low cohesion The second kind of smell,

    I call it:
  133. Slide № 133 “Cohesion” Related things stay together.

  134. Slide № 134 Example

  135. Slide № 135 A navigation bar Now, for our app,

    we need to create…
  136. Slide № 136 A navigation bar

  137. Slide № 137 class Navigation extends Component { render ()

    { } Using react-bootstrap, this should be straightforward.
  138. Slide № 138 class Navigation extends Component { render ()

    { return ( <Navbar> </Navbar> ) }
  139. Slide № 139 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> </Navbar> ) }
  140. Slide № 140 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> </Navbar> ) }
  141. Slide № 141 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> </Navbar> ) } So far, so good!
  142. Slide № 142 New feature!

  143. Slide № 143 New feature! Authentication system We need to

    update the navbar…
  144. Slide № 144 When logged out When logged in

  145. Slide № 145 When logged out When logged in

  146. Slide № 146 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> </Navbar> ) } }
  147. Slide № 147 class Navigation extends Component { 
 render

    () { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav>
  148. Slide № 148 class Navigation extends Component {
 static propTypes

    = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav>
  149. Slide № 149 class Navigation extends Component { static propTypes

    = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav>
  150. Slide № 150 const loggedIn = !!this.props.currentUser return ( <Navbar>

    <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> </Navbar> ) } }
  151. Slide № 151 const loggedIn = !!this.props.currentUser return ( <Navbar>

    <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> <Nav pullRight> </Nav> </Navbar> ) } }
  152. Slide № 152 const loggedIn = !!this.props.currentUser return ( <Navbar>

    <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> <Nav pullRight> {!loggedIn && <NavItem href='#' onClick={this.props.onLogin}>Login</NavI {loggedIn && <NavItem href='#' onClick={this.props.onLogout}>Logout</Nav </Nav> </Navbar> ) } }
  153. Slide № 153 class Navigation extends Component { static propTypes

    = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Now, Navigation is just a presentational component. We need to connect to the Redux store.
  154. Slide № 154 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header>
  155. Slide № 155 const enhance = connect( state => ({

    currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Now, looking at the component, we can see that cleanly separates into 3 concerns…
  156. Slide № 156 const enhance = connect( state => ({

    currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Selecting data from store
  157. Slide № 157 const enhance = connect( state => ({

    currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Selecting data from store Dispatching action to store
  158. Slide № 158 const enhance = connect( state => ({

    currentUser: selectCurrentUser(props) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Selecting data from store Dispatching action to store Rendering logic
  159. Slide № 159 New features!

  160. Slide № 160 Notification system Messaging system

  161. Slide № 161 When logged out When logged in

  162. Slide № 162 When logged out When logged in

  163. Slide № 163 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Coming back to our code…
  164. Slide № 164 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Selecting data from store Dispatching action to store Rendering logic We already see the pattern!
  165. Slide № 165 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> Selecting data from store Dispatching action to store Rendering logic Go with the flow!
  166. Slide № 166 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header>
  167. Slide № 167 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser
  168. Slide № 168 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser
  169. Slide № 169 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser
  170. Slide № 170 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends

    Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem>
  171. Slide № 171 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends

    Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header>
  172. Slide № 172 onViewMessages: MessagesIO.viewMessages } ) class Navigation extends

    Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header>
  173. Slide № 173 <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand>

    </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> <Nav pullRight> {!loggedIn && <NavItem href='#' onClick={this.props.onLogin}>Login</NavI {loggedIn && <NavItem href='#' onClick={this.props.onLogout}>Logout</Nav </Nav> </Navbar> ) } }
  174. Slide № 174 <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand>

    </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> </Nav> <Nav pullRight>
  175. Slide № 175 <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand>

    </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) </NavItem> } </Nav> <Nav pullRight>
  176. Slide № 176 <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand>

    </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) </NavItem> } {loggedIn && <NavItem href='/inbox' onClick={this.props.onViewMessages}> Messages ({this.props.newMessageCount}) </NavItem> } </Nav> <Nav pullRight>
  177. Slide № 177 <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand>

    </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) </NavItem> } {loggedIn && <NavItem href='/inbox' onClick={this.props.onViewMessages}> Messages ({this.props.newMessageCount}) </NavItem> } </Nav> <Nav pullRight>
  178. Slide № 178 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) Let’s zoom out a bit…
  179. Slide № 179 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) Selecting data from store Dispatching action to store Rendering logic Our code is still cleanly separated into 3 concerns!
  180. Slide № 180 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) Selecting data from store Dispatching action to store Rendering logic Cross-cutting concerns Cross-cutting concerns have to do with many different functionalities in our app.
  181. Slide № 181 User Notification Message User Message User Notification

    Message User Navigation Navigation User Navigation const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) But if we look from a functionality perspective… Many functional parts are intermixed inside the code: Hard to navigate.
  182. Slide № 182 User Notification Message User Message User Notification

    Message User Navigation Navigation User Navigation const enhance = connect( state => ({ currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) If we want to look at the code related to navigation we need to skip over a lot of unrelated code.
  183. Slide № 183 Real-world example

  184. Slide № 184 Bemuse options panel

  185. Slide № 185 Bemuse options panel

  186. Slide № 186 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) To do this, first, I have to add the code…
  187. Slide № 187 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) Here!
  188. Slide № 188 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) Here!
  189. Slide № 189 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) Skipping 80 lines, and…
  190. Slide № 190 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> + <OptionsPlayer.Row label="Gauge"> + <OptionsCheckbox + checked={Options.isGaugeEnabled(this.props.options)} + onToggle={this.props.onToggleGauge} + > + Show expert gauge <span className="OptionsPlayerͷhint">(experimental)</span> + </OptionsCheckbox> + </OptionsPlayer.Row> + <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) Here!
  191. Slide № 191 diff --git src/app/ui/OptionsPlayer.jsx src/app/ui/OptionsPlayer.jsx index a2e1d2e..2333c9f 100644

    --- src/app/ui/OptionsPlayer.jsx +++ src/app/ui/OptionsPlayer.jsx @@ -1,176 +1,191 @@ const enhance = compose( connect((state) => ({ options: state.options, scratch: Options.scratchPosition(state.options), })), connectIO({ onSetPanel: () => (value) => OptionsIO.setOptions({ 'player.P1.panel': value }), onSetScratch: () => (position) => OptionsIO.setScratch(position), onSetSpeed: () => (speed) => OptionsIO.setSpeed(speed), onSetLeadTime: () => (leadTime) => OptionsIO.setLeadTime(leadTime), onSetLaneCover: () => (laneCover) => OptionsIO.setLaneCover(laneCover), onToggleBackgroundAnimationsEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'system.bga.enabled': Options.toggleOption(options['system.bga.enabled']) }) ), onToggleAutoVelocityEnabled: ({ options }) => () => ( OptionsIO.setOptions({ 'player.P1.auto-velocity': Options.toggleOption(options['player.P1.auto-velocity']) }) + ), + onToggleGauge: ({ options }) => () => ( + OptionsIO.setOptions({ + 'player.P1.gauge': Options.toggleGauge(options['player.P1.gauge']) + }) ) }), pure ) export const OptionsPlayer = React.createClass({ propTypes: { options: React.PropTypes.object, scratch: React.PropTypes.string, onClose: React.PropTypes.func, onSetPanel: React.PropTypes.func, onSetScratch: React.PropTypes.func, onSetSpeed: React.PropTypes.func, onSetLaneCover: React.PropTypes.func, onToggleBackgroundAnimationsEnabled: React.PropTypes.func, onToggleAutoVelocityEnabled: React.PropTypes.func, + onToggleGauge: React.PropTypes.func, onSetLeadTime: React.PropTypes.func }, render () { return <div className="OptionsPlayer"> <OptionsPlayer.Row label="Speed" hidden={Options.isAutoVelocityEnabled(this.props.options)} > <OptionsSpeed value={this.props.options['player.P1.speed']} onChange={this.props.onSetSpeed} /> <div className="OptionsPlayerͷhelp"> You can also change the speed in-game<br />using the Up and Down arrow keys. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="LeadTime" hidden={!Options.isAutoVelocityEnabled(this.props.options)} > <OptionsLeadTimeInputField value={Options.leadTime(this.props.options)} onChange={this.props.onSetLeadTime} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp"> Speed will be automatically adjusted<br />to maintain a consistent note velocity. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="Scratch"> <OptionsPlayerSelector type="scratch" options={SCRATCH_OPTIONS} onSelect={this.props.onSetScratch} value={this.props.scratch} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Panel"> <OptionsPlayerSelector type="panel" options={PANEL_OPTIONS} onSelect={this.props.onSetPanel} value={this.props.options['player.P1.panel']} /> </OptionsPlayer.Row> <OptionsPlayer.Row label="Cover"> <OptionsLaneCoverInputField value={Options.laneCover(this.props.options)} onChange={this.props.onSetLaneCover} style={{ width: '5em' }} /> <div className="OptionsPlayerͷhelp" title="Can be negative, in this case the play area is pulled up." > The amount of play area to hide from the top. </div> </OptionsPlayer.Row> <OptionsPlayer.Row label="BGA"> <OptionsCheckbox checked={Options.isBackgroundAnimationsEnabled(this.props.options)} onToggle={this.props.onToggleBackgroundAnimationsEnabled} > Enable background animations <span className="OptionsPlayerͷhint">(720p, alpha)</span> </OptionsCheckbox> </OptionsPlayer.Row> <OptionsPlayer.Row label="AutoVel"> <OptionsCheckbox checked={Options.isAutoVelocityEnabled(this.props.options)} onToggle={this.props.onToggleAutoVelocityEnabled} > Maintain absolute note velocity <span className="OptionsPlayerͷhint">(advanced)</span> </OptionsCheckbox> </OptionsPlayer.Row> + <OptionsPlayer.Row label="Gauge"> + <OptionsCheckbox + checked={Options.isGaugeEnabled(this.props.options)} + onToggle={this.props.onToggleGauge} + > + Show expert gauge <span className="OptionsPlayerͷhint">(experimental)</span> + </OptionsCheckbox> + </OptionsPlayer.Row> + <div className="OptionsPlayerͷbuttons"> <OptionsButton onClick={this.props.onClose}>Save & Exit</OptionsButton> </div> </div> } }) As I add more options things are going to be more spread apart.
  192. Slide № 192 Cohesion loss Symptom: Making a simple change

    requires changing code that is further apart.
  193. Slide № 193 Cohesion loss Symptom: Reviewing a PR requires

    a lot of scrolling up and down to understand.
  194. Slide № 194 components/ scoreboard/ ScoreboardView.js modules/ constants.js scoreboard/ actions.js

    reducer.js selectors.js utils.js This can happen when files are organized by type. To add a new feature, I need to change 6 files.
  195. Slide № 195 While grouping files this way makes the

    architecture more obvious (this may be very suitable for small projects)
  196. Slide № 196 200 files 200 files 200 files 500

    files But it may not scale very well to hundreds of files per type.
  197. Slide № 197 Let’s try aiming for functional cohesion instead

    of logical cohesion. Instead of “this part selects data from the store”, “this part dispatches”, etc… No. We make it: “this part for messages”, “this part for notifications”, etc.
  198. Slide № 198 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func
  199. Slide № 199 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func Remove stuff not directly related to Navigation
  200. Slide № 200 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func
  201. Slide № 201 const enhance = connect( state => ({

    currentUser: selectCurrentUser(state), newNotificationCount: selectNewNotificationCount(state), newMessageCount: selectNewMessageCount(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout, onViewMessages: MessagesIO.viewMessages } ) class Navigation extends Component { static propTypes = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func
  202. Slide № 202 class Navigation extends Component { static propTypes

    = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem>
  203. Slide № 203 class Navigation extends Component { static propTypes

    = { currentUser: PropTypes.object, onLogin: PropTypes.func, onLogout: PropTypes.func, newNotificationCount: PropTypes.number, newMessageCount: PropTypes.number, onViewMessages: PropTypes.func } render () { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem>
  204. Slide № 204 class Navigation extends Component { render ()

    { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) </NavItem> } {loggedIn && <NavItem href='/inbox' onClick={this.props.onViewMessages}>
  205. Slide № 205 class Navigation extends Component { render ()

    { const loggedIn = !!this.props.currentUser return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> {loggedIn && <NavItem href='/notifications'> Notifications ({this.props.newNotificationCount}) </NavItem> } {loggedIn && <NavItem href='/inbox' onClick={this.props.onViewMessages}> Replace the parts that require special logic with specialized components.
  206. Slide № 206 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> <NotificationsNavItem /> <MessagesNavItem />
  207. Slide № 207 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> )
  208. Slide № 208 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> ) Example: The LoginLogoutNavItem would know how to figure out the current authentication state on its own.
  209. Slide № 209 const LoginLogoutNavItem = Let’s look at it…

  210. Slide № 210 const LoginLogoutNavItem = connect( )

  211. Slide № 211 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), )
  212. Slide № 212 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )
  213. Slide № 213 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) })
  214. Slide № 214 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) This looks pretty good! Most of the code is now concerned with auth and current user.
  215. Slide № 215 User Navigation Navigation const LoginLogoutNavItem = connect(

    state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) Except for 2 lines concerned with navigation:
  216. Slide № 216 User Navigation Navigation const LoginLogoutNavItem = connect(

    state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) If, in the future, we need to replace the NavItem with something else…
  217. Slide № 217 User Navigation Navigation const LoginLogoutNavItem = connect(

    state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) We have to change not only Navigation, but all components used by Navigation that renders a NavItem.
  218. Slide № 218 User Navigation Navigation const LoginLogoutNavItem = connect(

    state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) What can we do here?
  219. Slide № 219 User Navigation Navigation const LoginLogoutNavItem = connect(

    state => ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) Delegate rendering logic
  220. Slide № 220 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( <NavItem href='#' onClick={props.onLogout}>Logout</NavItem> ) : ( <NavItem href='#' onClick={props.onLogin}>Login</NavItem> ) ) }) Instead of hardcoding the rendering logic here…
  221. Slide № 221 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) …let the component users supply its own rendering logic.
  222. Slide № 222 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) })
  223. Slide № 223 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) })
  224. Slide № 224 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  225. Slide № 225 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  226. Slide № 226 const LoginLogoutNavItem = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } User Only concerned about current user and authentication Not concerned about how to render it
  227. Slide № 227 User const LoginLogoutNavItem = connect( state =>

    ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function LoginLogoutNavItem (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) LoginLogoutNavItem.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } Let’s change the name…
  228. Slide № 228 User const Auth = connect( state =>

    ({ currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } The Auth component can now be reused in other parts of the app. (e.g. showing a piece of content only when the user is logged in)
  229. Slide № 229 const Auth = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  230. Slide № 230 const Auth = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } “Render props” This technique is now officially known as:
  231. Slide № 231 Back to our <Navigation />

  232. Slide № 232 class Navigation extends Component { render ()

    { return ( <Navbar> <Navbar.Header> <Navbar.Brand> <a href='#'>My app</a> </Navbar.Brand> </Navbar.Header> <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem> <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> )
  233. Slide № 233 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> ) } }
  234. Slide № 234 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <LoginLogoutNavItem /> </Nav> </Navbar> ) } }
  235. Slide № 235 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <Auth /> </Nav> </Navbar> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  236. Slide № 236 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <Auth renderLoggedOut={ } renderLoggedIn={ } /> </Nav> </Navbar> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  237. Slide № 237 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <Auth renderLoggedOut={(login) => <NavItem href='#' onClick={login}>Login</NavItem> } renderLoggedIn={(user, logout) => <NavItem href='#' onClick={logout}>Logout</NavItem> } /> </Nav> </Navbar> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  238. Slide № 238 <Nav> <NavItem href='/'>Home</NavItem> <NavItem href='/explore'>Explore</NavItem> <NavItem href='/search'>Search</NavItem>

    <NotificationsNavItem /> <MessagesNavItem /> </Nav> <Nav pullRight> <Auth renderLoggedOut={(login) => <NavItem href='#' onClick={login}>Login</NavItem> } renderLoggedIn={(user, logout) => <NavItem href='#' onClick={logout}>Logout</NavItem> } /> </Nav> </Navbar> ) } } Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  239. Slide № 239 <Auth />

  240. Slide № 240 const Auth = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired }
  241. Slide № 241 const Auth = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } Objective: Connect UI to data source.
  242. Slide № 242 const Auth = connect( state => ({

    currentUser: selectCurrentUser(state) }), { onLogin: LoginIO.showLoginModal, onLogout: LoginIO.logout } )(function Auth (props) { return ( props.currentUser ? ( props.renderLoggedIn(props.currentUser, props.onLogout) ) : ( props.renderLoggedOut(props.onLogin) ) ) }) Auth.propTypes = { renderLoggedIn: PropTypes.func.isRequired, renderLoggedOut: PropTypes.func.isRequired } “Data connector component” At Taskworld, we call this kind of components:
  243. Slide № 243 Navigation Recap

  244. Slide № 244 Navigation As components grow bigger it can

    be hard to maintain.
  245. Slide № 245 NavigationContainer NavigationView The most obvious way to

    split them. But this could sometimes lead to cohesion loss, especially when one part would not make sense without the other.
  246. Slide № 246 Navigation Auth Unread MessagesData Unread NotificationsData We

    can try to split by functional goals:
  247. Slide № 247 Navigation Auth Unread MessagesData Unread NotificationsData If

    done right, it’s also easier to re-use these components in other parts of the app.
  248. Slide № 248 Conclusions

  249. Slide № 249 When code reviewing Consistency Readability Logical correctness

    We usually look for: These are relatively simpler to review.
  250. Slide № 250 When code reviewing Consistency Readability Logical correctness

    Smells Subtle. It’s easy to see the code as “correct” but we may miss the subtle design issues in our code, unless we’re aware of them. To build maintainable software, I encourage everyone to develop the sense of smells and be mindful of them!
  251. Slide № 251 I don’t know of any way to

    detect these smells automatically. Today I showed you 2 kinds of smells. These are the kinds of smells I encountered the most often.
  252. Slide № 252 When writing/reviewing code, try looking for these

    smells.
  253. Slide № 253 Not all smells are bad. Last but

    not least…
  254. Slide № 254 Not all smells are bad. Example: Duplicated

    code a well known code smell
  255. Slide № 255 Not all smells are bad. Example: Duplicated

    code can sometimes be easier to read because they are more explicit and require less amount of abstraction
  256. Slide № 256 That’s why I always duplicate my slides

    instead of using animations!
  257. Slide № 257 Everything is a trade-off. In the end,

    Examples: Smaller modules → More modules. Reducing amount of code you maintain → More 3rd party dependencies. We have to make a lot of choices. We have to strike a good balance.
  258. Slide № 258 Look for feedback. To see if we

    made the right trade-offs, After all, we make code maintainable so that other people can maintain it. So listen to them — look for their feedback!
  259. Slide № 259 Thank you!