Skip to content

Commit

Permalink
Small improvements
Browse files Browse the repository at this point in the history
- Trim search query - obvious in hindsight. Does away with the need to set isSearching to false if the search
query is falsy.
- Rename test spec + add inline snapshot comparison that checks for the existence of forecast items.
  • Loading branch information
denniskigen committed Jul 4, 2021
1 parent 59e3402 commit ffc176e
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 18 deletions.
20 changes: 16 additions & 4 deletions src/__tests__/app.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import App from '../components/app';

describe('<App />', () => {
beforeAll(() => jest.useFakeTimers());

afterAll(() => jest.clearAllTimers());

const history = createMemoryHistory();
Expand All @@ -24,7 +23,7 @@ describe('<App />', () => {
</Router>,
);

test('renders the app', async () => {
test('fetches and renders the current weather and a five day forecast', async () => {
renderApp();

await waitForLoadingToFinish();
Expand All @@ -36,15 +35,28 @@ describe('<App />', () => {
expect(
screen.getByPlaceholderText(/search for a location/i),
).toBeInTheDocument();
await screen.findByText(/eldoret, ke/i);

expect(screen.getByText(/eldoret, ke/i)).toBeInTheDocument();
expect(screen.getByText(/broken clouds/i)).toBeInTheDocument();
expect(screen.getByText(/feels like 18°/i)).toBeInTheDocument();
expect(screen.getByText(/30m\/s winds/i)).toBeInTheDocument();
expect(screen.getByText(/49% humidity/i)).toBeInTheDocument();
expect(
screen.getByText(/'Netflix and chill' weather. It's pleasant outside/i),
).toBeInTheDocument();
expect(screen.getAllByRole('list').length).toEqual(5);
expect(screen.getAllByRole('listitem').length).toEqual(5);
const forecast = screen.getAllByRole('listitem').map((listItem) => {
return listItem.textContent;
});
expect(forecast).toMatchInlineSnapshot(`
Array [
"Saturday22° / 22°",
"Sunday22° / 22°",
"Monday22° / 22°",
"Tuesday20° / 20°",
"Wednesday21° / 21°",
]
`);
expect(screen.getByText(/Open source by/i)).toBeInTheDocument();
expect(screen.getByText(/Dennis Kigen/i)).toBeInTheDocument();
expect(screen.getByText(/©2020 - now/i)).toBeInTheDocument();
Expand Down
26 changes: 12 additions & 14 deletions src/components/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,8 @@ function viewStateReducer(state, action) {
const App = () => {
const [location, setLocation] = React.useState('Eldoret');
const [debouncedSearchTerm, setDebouncedSearchTerm] = React.useState('');
const [isSearching, setIsSearching] = React.useState(false);
const [units, setUnits] = React.useState('metric');
const [state, dispatch] = React.useReducer(viewStateReducer, {
const [viewState, dispatch] = React.useReducer(viewStateReducer, {
status: 'started',
error: null,
forecast: [],
Expand All @@ -59,12 +58,11 @@ const App = () => {
);

const handleLocationChange = (event) => {
if (event.target.value) {
const query = event.target.value.trim();
if (query) {
setIsSearching(true);
} else {
setIsSearching(false);
}
debounceSearch(event.target.value);
debounceSearch(query);
};

const handleUnitsChange = (newUnits) => {
Expand All @@ -75,7 +73,7 @@ const App = () => {
if (debouncedSearchTerm) {
setLocation(debouncedSearchTerm);
}
}, [debouncedSearchTerm, isSearching]);
}, [debouncedSearchTerm,]);

React.useEffect(() => {
async function getWeather() {
Expand Down Expand Up @@ -129,8 +127,8 @@ const App = () => {
<NavBar />
<Switch>
<Route exact path="/">
{state.status === 'started' ? <Loading /> : null}
{state.status === 'rejected' ? (
{viewState.status === 'started' ? <Loading /> : null}
{viewState.status === 'rejected' ? (
<div className="w-3/5 md:w-3/5 lg:w-1/2 m-auto">
<div className="mx-auto sm:max-w-xl 2xl:max-w-2xl">
<div
Expand All @@ -150,14 +148,14 @@ const App = () => {
fillRule="evenodd"
/>
</svg>
<span>{state.error.message}</span>
<span>{viewState.error.message}</span>
</div>
</div>
</div>
</div>
) : null}
{(state.weather && Object.keys(state.weather).length) ||
(state.forecast && Object.keys(state.forecast).length) ? (
{(viewState.weather && Object.keys(viewState.weather).length) ||
(viewState.forecast && Object.keys(viewState.forecast).length) ? (
<main>
<div className="mx-auto w-5/6 md:w-full 2xl:max-w-7xl xl:max-w-6xl">
<Search
Expand All @@ -166,8 +164,8 @@ const App = () => {
onLocationChange={handleLocationChange}
/>
<WeatherCard
forecast={state.forecast}
weather={state.weather}
forecast={viewState.forecast}
weather={viewState.weather}
units={units}
onUnitsChange={handleUnitsChange}
/>
Expand Down

1 comment on commit ffc176e

@vercel
Copy link

@vercel vercel bot commented on ffc176e Jul 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.