App #51
App #51
Conversation
There was a problem hiding this comment.
The structure of the project looks fine, I can see you tried to follow the guidelines we gave you to make the code structure readable. However, as there are hardly any comments in the code that makes understanding and even reviewing the code very cumbersome. Please provide documentation in the code for all public API and inline comments in the code explaining what the code does in some parts. Try to make that a habit to write comments as you write the code, even better: write the documentation of what a method is supposed to do before writing its first line of code. That makes you better aware of what you want to write and has the benefit that the documentation then is already there.
Please start commenting some parts an re-request a review on those parts so that we can have a closer look at that.
| import * as React from "react" | ||
| import Svg, { SvgProps, Path } from "react-native-svg" | ||
| const LesserLevelCrossing = (props: SvgProps) => ( | ||
| <Svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| width={46} | ||
| height={40} | ||
| viewBox="0 0 46 40" | ||
| fill="none" | ||
| {...props} | ||
| > | ||
| <Path | ||
| fill="#FAFAFA" | ||
| stroke="#DE0B0B" | ||
| strokeWidth={4} | ||
| d="M23 37.57 2.464 2h41.072L23 37.57Z" | ||
| /> | ||
| </Svg> | ||
| ) | ||
| export default LesserLevelCrossing |
There was a problem hiding this comment.
Is it common to build SVG assets like this for react? For easier editing and handling of these assets I would assume saving them as .svg files and including in some way would be better.
There was a problem hiding this comment.
I will look out for a better way of implementing svg support, but this is at least a very common way to use svgs (https://stackoverflow.com/a/52663626)
| const styles = StyleSheet.create({ | ||
| primary: { | ||
| borderRadius: 50, | ||
| padding: 15, | ||
| backgroundColor: Color.primary, | ||
| }, | ||
| secondary: { | ||
| borderRadius: 50, | ||
| padding: 15, | ||
| }, | ||
| disabled: { | ||
| borderRadius: 50, | ||
| padding: 15, | ||
| backgroundColor: Color.darkGray, | ||
| }, | ||
| textPrimary: { | ||
| color: Color.textLight, | ||
| fontSize: 18, | ||
| textAlign: "center", | ||
| }, | ||
| textSecondary: { | ||
| color: Color.primary, | ||
| fontSize: 18, | ||
| textAlign: "center", | ||
| }, | ||
| textDisabled: { | ||
| color: Color.darkGray, | ||
| fontSize: 18, | ||
| textAlign: "center", | ||
| }, |
There was a problem hiding this comment.
This looks like something that should be css classes and used as such in the structure. Does the framework support css or css-like general stylings that can be reused?
There was a problem hiding this comment.
This is the css-like styling of React. I use reusable stylings for styles that should be applied in the whole app (see values/text-styles.ts). Since these styles are button specific, they are part of the button component, the non specific parts (eg. colors) of these styles are in general styling files.
No description provided.