Skip to content

Instantly share code, notes, and snippets.

@TimCodes
Created September 5, 2020 00:35
Show Gist options
  • Save TimCodes/df7b093bca4e05421533fbb1e996b0f3 to your computer and use it in GitHub Desktop.
Save TimCodes/df7b093bca4e05421533fbb1e996b0f3 to your computer and use it in GitHub Desktop.
/*
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