Created
September 5, 2020 00:35
-
-
Save TimCodes/df7b093bca4e05421533fbb1e996b0f3 to your computer and use it in GitHub Desktop.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| /* | |
| Prompt: | |
| We have defined a basic dropdown via the Dropdown and DropdownItem components below, with example usage in the ExampleNav component. | |
| The Dropdown and DropdownItem components have some problems, and also have room for improvements. | |
| Please fix any obvious bugs you see with the dropdown, and explain your reasoning. | |
| Please then describe some improvements you would make to the dropdown, and how you would implement them. | |
| Consider the different contexts in which you might use this dropdown and what changes might be neccessary to make it more flexible. | |
| Follow up question: if we wanted to sync this dropdown selection to the server with app.sync('PATCH', 'user', { dropdown_1_state: {true,false} }) where would this be included | |
| PS: No need to worry about CSS. | |
| */ | |
| import React, {PureComponent} from 'react'; | |
| /* | |
| **** Possible Improvements **** | |
| I alwasy try to balance between complexity and flexability | |
| // Simple version with less flexability | |
| Adding a menuItems prop and a onItemClick prop would | |
| cover most use cases and give a simple component api. | |
| I would keep the ability to render children as well | |
| // example | |
| <Dropdown | |
| menuItems = {[{value : 'value', title :'title'}, ...] } | |
| onItemClick = { value => setSomeState(value) } | |
| /> | |
| // More Complex version with better flexability | |
| Adding props to control the classes and the container element. | |
| would give more styling versality. Having a prop that lets | |
| you control when the dropdown is open would give you greater control | |
| // example | |
| <Dropdown | |
| menuItems = {[{value : 'value', title :'title'}, ...] } | |
| onItemClick = { value => setSomeState(value) } | |
| isOpen = {this.state.isDDOpen} | |
| onClick = { e => this.openDD(e)} | |
| openClassNmae = 'dd__open' | |
| containerEl = <MyCustomDDContainer /> | |
| /> | |
| */ | |
| /* | |
| **** Server Sync **** | |
| The logic to call the server should be encapsulated | |
| in a seperate piece of code, such as a thunk or a API service. | |
| The dropdown component should then call that code when its toggled | |
| */ | |
| class Dropdown extends PureComponent { | |
| /* | |
| Using Class properties fixes the issues with THIS, and | |
| looks cleaner to me | |
| */ | |
| state = { isOpen: false } | |
| toggle= () => { | |
| // I feel that when accessing previous state, using the | |
| // setState function is more idiomatic | |
| this.setState(prevState => ({ isOpen: !prevState.isOpen })); | |
| } | |
| render() { | |
| const {isOpen} = this.state; | |
| const {label} = this.props; | |
| return ( | |
| <div className="dropdown"> | |
| <button type="button" className="dropdown-button" id="dropdownButton" aria-haspopup="true" aria-expended={isOpen} onClick={this.toggle}>{label}</button> | |
| <ul className={`${isOpen ? 'dropdown-open' : ''} dropdown-menu`} aria-labelledby="dropdownButton" role="menu"> | |
| { | |
| // simple solution to only showing items when isOpen is true | |
| isOpen && this.props.children | |
| } | |
| </ul> | |
| </div> | |
| ); | |
| } | |
| } | |
| class DropdownItem extends PureComponent { | |
| // Returning children here is a simple solution | |
| // and reflects how the component is being used | |
| render() { | |
| return this.props.children | |
| } | |
| } | |
| class ExampleNav extends PureComponent { | |
| render() { | |
| return ( | |
| <nav> | |
| <a href="/page1">Page 1</a> | |
| <Dropdown label="More items"> | |
| <DropdownItem href="/page2">Page 2</DropdownItem> | |
| <DropdownItem href="/page3">Page 3</DropdownItem> | |
| <DropdownItem href="/page4">Page 4</DropdownItem> | |
| </Dropdown> | |
| <Dropdown label="Even more items"> | |
| <DropdownItem href="/page5">Page 5</DropdownItem> | |
| <DropdownItem href="/page6">Page 6</DropdownItem> | |
| </Dropdown> | |
| </nav> | |
| ); | |
| } | |
| } |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment