Skip to content

Added card, format changes#32

Open
walla2jd wants to merge 1 commit intomasterfrom
walla2jdCodeReview2
Open

Added card, format changes#32
walla2jd wants to merge 1 commit intomasterfrom
walla2jdCodeReview2

Conversation

@walla2jd
Copy link
Collaborator

@walla2jd walla2jd commented Apr 3, 2022

Analysis of the program:
Was the program available in UC Github on time?
Yes
Is the program documented/commented well enough for you to understand?
Yes
Does the program compile?
Yes
Rationale behind your changes.
The text in the outlined text fields were cutting off for me so adding the Card fixed that. The rest were removing unnecessary parentheses and changing from var to val.
Links to three specific commits that you made to your own group's GitHub repository after Sprint 1's deadline, and before Sprint 2's deadline.
ahlersml/MyCleaningLog@9a344d2
ahlersml/MyCleaningLog@67097ed
ahlersml/MyCleaningLog@e44cba9

Technical Concepts
I learned more about working with the camera.
Working with different Card properties such as OutlinedTextField
Learned more about state holders ( by remember { mutableStateOf("") } )

var permissionGranted = false
resultsMap.forEach {
if (it.value == true) {
if (it.value) {

Choose a reason for hiding this comment

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

A good suggestion, though it appeared in a previous review. When that happens, first in wins.

fun fetchIncidents(fromCaseYear: Int, toCaseYear: Int, state: Int, county: Int){
viewModelScope.launch {
var innerIncidents = incidentService.fetchIncidents(fromCaseYear, toCaseYear, state, county)
val innerIncidents = incidentService.fetchIncidents(fromCaseYear, toCaseYear, state, county)

Choose a reason for hiding this comment

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

A good suggestion, though it appeared in a previous review. When that happens, first in wins.

super.onCreate(savedInstanceState)
setContent {
FreeWaysTheme() {
FreeWaysTheme{

Choose a reason for hiding this comment

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

A good suggestion, though it appeared in a previous review. When that happens, first in wins.

way2 = inWay2
vehiclesInvolved = 0

Card {

Choose a reason for hiding this comment

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

Are you adding styling to the Card? If not, is the card adding value?

@discospiff
Copy link

Some good suggestions here, but most were covered in previous reviews. No need to merge the same change more than once.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants