Add app content #2
Conversation
| @@ -6,6 +6,7 @@ | |||
| "scripts": { | |||
| "start": "webpack-dev-server --open --mode development", | |||
| "test": "jest", | |||
| "eslint": "eslint './**/*.{js,jsx}'", | |||
There was a problem hiding this comment.
I copied this from the example in some previous module to keep things as familiar as possible for the students.
public/index.html
Outdated
| <html lang="en"> | ||
| <head> | ||
| <meta charset="utf-8"> | ||
| <title>Pokemons</title> |
There was a problem hiding this comment.
The plural of pokemon is just pokemon ;D
There was a problem hiding this comment.
I should've known better! 😱
src/Loader.jsx
Outdated
| @@ -0,0 +1,7 @@ | |||
| import React from 'react' | |||
|
|
|||
| const Loader = () => ( | |||
There was a problem hiding this comment.
Maybe call this LoadingSpinner? Loader sounds like something that loads something
src/PokemonPage.jsx
Outdated
| <div className="pokemon-ability"> | ||
| <div className="pokemon-ability-type">Ability</div> | ||
| <div className="pokemon-ability-name"> | ||
| {normalAbility.ability.name.replace('-', ' ')} |
There was a problem hiding this comment.
We could extract the dash removing to a helper function, we now do it in three different places. Maybe something like formatAbilityName(nameWithDash: string): string that would do just that
src/PokemonPage.jsx
Outdated
| <div className="pokemon-ability"> | ||
| <div className="pokemon-ability-type">Hidden ability</div> | ||
| <div className="pokemon-ability-name"> | ||
| {hiddenAbility.ability.name.replace('-', ' ')} | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
This is a duplicate of the normalAbility We can extract these to its own component that takes the title and the abilityName as props
| .pokemon-type-bug { | ||
| --pokemon-type-color: #ac2; | ||
| } | ||
|
|
||
| .pokemon-type-dark { | ||
| --pokemon-type-color: #754; | ||
| } | ||
|
|
||
| .pokemon-type-dragon { | ||
| --pokemon-type-color: #73f; | ||
| } |
There was a problem hiding this comment.
Did you do these by hand? All the colors? :O
| const [error, setError] = useState() | ||
| useEffect(() => { | ||
| setIsLoading(true) | ||
| axios |
There was a problem hiding this comment.
Just curious, why use a library when we could do this with window.fetch?
There was a problem hiding this comment.
They were using axios in the React module so I decided to stick with it.
There was a problem hiding this comment.
Yes, that is then good enough for us as well!
| await act(async () => { | ||
| render(<App />) | ||
| }) | ||
| expect(axiosMock.get).toHaveBeenCalledTimes(1) |
There was a problem hiding this comment.
We could check the params of what get was called with
test/App.jest.spec.jsx
Outdated
| }) | ||
|
|
||
| it('shows error', async () => { | ||
| axiosMock.get.mockResolvedValueOnce(Promise.reject(new Error())) |
There was a problem hiding this comment.
you can also do .mockRejectedValueOnce!
src/PokemonPage.jsx
Outdated
| const previous = pokemons.find(({ id }) => id === pokemon.id - 1) | ||
| const next = pokemons.find(({ id }) => id === pokemon.id + 1) |
There was a problem hiding this comment.
I feel like these should come in as props. A page for a single Pokemon should really not consume ALL the pokemon.
Glad you like it! I made the card based on some design on the Internet. I cleaned up the code based on your comments 👍 |
test/PokemonPage.jest.spec.jsx
Outdated
| <PokemonPage pokemons={pokemons} /> | ||
| <PokemonPage pokemonList={pokemons} /> |
There was a problem hiding this comment.
The value of the prop is still pokemons 🙈
Fix bad formating caused by env
Exercise 11.13 smartlyio#2
A simple Pokedex app with two levels of navigation. I've attempted to use the same libs, lint rules etc. as in previous modules to not create extra confusion.
We could add a search filed to the list page if we want more stuff.
There's jest and eslint, and a few tests. We can break some of those later.