Sara Boström Bundock - Week 16- React Native app #298
Conversation
bostrombundock
changed the title
There was a problem hiding this comment.
Nice work this week getting introduced to React Native! I had a few comments on better practices or alternatives to your approach just to give a bit of a different perspective. Your app generates a new quote both by shaking and by pressing the button, as it should. Sometimes it looks a bit wonky, like maybe two quotes are generated when shaking but I'm not sure if that's me the user or the code, and I don't have any suggestions for looking into it.
Unrelated to React Native, a comment on commit messages: I think of making a commit every time I accomplish something, and try practicing writing the “why” behind the changes as your commit message. Git tracks the actual code changes, so that is easy to see. But in the real world if someone were to look at your changes, the message is an opportunity for a little context. So ex. instead of writing “Style added” you could write “Width added for better responsiveness on mobile view."
Keep up the good work! :)
| import React, { useState, useEffect } from "react"; | ||
| import { Accelerometer } from "expo-sensors"; | ||
| import styled from "styled-components/native"; | ||
| import { View, Text, TouchableOpacity } from "react-native"; |
There was a problem hiding this comment.
TouchableOpacity is never used, so it could be removed.
| export const SensorComponent = () => { | ||
| // This function determines how often our program reads the accelerometer data in milliseconds | ||
| // https://docs.expo.io/versions/latest/sdk/accelerometer/#accelerometersetupdateintervalintervalms | ||
| const [quote, setQuote] = useState({}); |
There was a problem hiding this comment.
It's good practice to keep all of your state together at the top of your component, so instead of having three spread out through your code, group them here, like so:
const [quote, setQuote] = useState({});
const [subscription, setSubscription] = useState(null);
const [data, setData] = useState({
x: 0,
y: 0,
z: 0,
});
components/SensorComponent.js
Outdated
| flex-direction: column; | ||
| text-align: center; | ||
| margin-bottom: 20px; | ||
| overflow-wrap:break-word; |
There was a problem hiding this comment.
When I try to run the app on Expo Go on mobile, it throws an error about this particular style overflow-wrap. Maybe it's not React Native compatible, I'm not sure. I just commented it out and the app runs fine.
| margin-left: 20px; | ||
| margin-right:20px; |
There was a problem hiding this comment.
An alternative to writing out each margin is to group them in the order top, right, bottom, left (clockwise). If you want zero margin, you just write 0 no need to write units.
// margins can be written margin: top right bottom left
// which in your case looks like below:
margin: 0 20px 20px 20px;
But the way you've done it also works perfectly fine!
2 participants
My reflections on how this week's project turned out:
A short and intense project with a whole new concept React native.