david/poker-chips-helper
1
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Ability to save and load calculator states into two slots #37

Merged
MantashaNoyela merged 4 commits from MantashaNoyela/26 into main 2025-02-25 23:28:55 -08:00
Conversation 1 Commits 4 Files Changed 5 +160 -25
MantashaNoyela commented 2025-02-25 20:35:58 -08:00 (Migrated from github.com)
Copy Link

This file is responsible for managing the saving and loading of poker calculator states into two storage slots using AsyncStorage in React Native. It provides functions to persist and retrieve game-related data, such as the number of players, buy-in amount, and chip distribution, allowing users to load and save game states when needed.

This file is responsible for managing the saving and loading of poker calculator states into two storage slots using AsyncStorage in React Native. It provides functions to persist and retrieve game-related data, such as the number of players, buy-in amount, and chip distribution, allowing users to load and save game states when needed.
djwesty (Migrated from github.com) requested changes 2025-02-25 21:26:59 -08:00
app/index.tsx
djwesty (Migrated from github.com) commented 2025-02-25 21:23:25 -08:00
Copy Link

It appears that when resolving the merge conflicts, there may have been some regressions. That is to say, these things were changed and committed before you started work, but then undone here. Things I notice on quick glace

  • This title "Poker chips helper" was removed
  • The default state of buyInAmount near the top was changed to 20 from null

Can you fix these up?

It appears that when resolving the merge conflicts, there may have been some regressions. That is to say, these things were changed and committed before you started work, but then undone here. Things I notice on quick glace - This title "Poker chips helper" was removed - The default state of `buyInAmount` near the top was changed to `20` from `null` Can you fix these up?
djwesty (Migrated from github.com) commented 2025-02-25 21:26:34 -08:00
Copy Link

These buttons at the end are ok and satisfy the issue. In the next sprint, we should explore moving them to the title bar and maybe replacing them with an icon or something similar.

Nothing to do for now, but keep a mental note

These buttons at the end are ok and satisfy the issue. In the next sprint, we should explore moving them to the title bar and maybe replacing them with an icon or something similar. Nothing to do for now, but keep a mental note
components/CalculatorState.tsx
@ -0,0 +28,4 @@
} catch (error) {
return null;
}
};
djwesty (Migrated from github.com) commented 2025-02-25 21:18:28 -08:00
Copy Link

From what I can tell, this file seems to be a set of helper functions and interface like things. Since the file is not a React component, we can organize it a little better

  • The file should probably be in another location, not components. One idea is to put it in a new directory like util
  • The file can just use the extension .ts not .tsx
From what I can tell, this file seems to be a set of helper functions and interface like things. Since the file is not a React component, we can organize it a little better - The file should probably be in another location, not `components`. One idea is to put it in a new directory like `util` - The file can just use the extension `.ts` not `.tsx`
package.json
djwesty (Migrated from github.com) commented 2025-02-25 21:24:40 -08:00
Copy Link

Another possible regression here. Can you undo this one please?

Another possible regression here. Can you undo this one please?
djwesty commented 2025-02-25 21:28:26 -08:00 (Migrated from github.com)
Copy Link

Looks pretty good overall, but a few small changes are needed. I went ahead and testing, and the functionality seems great. Good work

Looks pretty good overall, but a few small changes are needed. I went ahead and testing, and the functionality seems great. Good work
MantashaNoyela (Migrated from github.com) reviewed 2025-02-25 22:03:04 -08:00
app/index.tsx
MantashaNoyela (Migrated from github.com) commented 2025-02-25 22:03:04 -08:00
Copy Link

As I encountered the conflict problem, I removed the folder from my laptop and cloned it again. But I used the code which I had written way before that. May be that's why these problem occured.

As I encountered the conflict problem, I removed the folder from my laptop and cloned it again. But I used the code which I had written way before that. May be that's why these problem occured.
MantashaNoyela (Migrated from github.com) reviewed 2025-02-25 22:04:06 -08:00
package.json
MantashaNoyela (Migrated from github.com) commented 2025-02-25 22:04:06 -08:00
Copy Link

Delete the package.jason file?

Delete the package.jason file?
MantashaNoyela (Migrated from github.com) reviewed 2025-02-25 22:04:33 -08:00
app/index.tsx
MantashaNoyela (Migrated from github.com) commented 2025-02-25 22:04:33 -08:00
Copy Link

got it

got it
MantashaNoyela (Migrated from github.com) reviewed 2025-02-25 22:05:22 -08:00
components/CalculatorState.tsx
@ -0,0 +28,4 @@
} catch (error) {
return null;
}
};
MantashaNoyela (Migrated from github.com) commented 2025-02-25 22:05:22 -08:00
Copy Link

Do I have to pull request again?

Do I have to pull request again?
Vutukuri15 (Migrated from github.com) reviewed 2025-02-25 22:10:38 -08:00
components/__tests__/CalculatorState.test.tsx
Vutukuri15 (Migrated from github.com) commented 2025-02-25 22:06:36 -08:00
Copy Link

Good work, Well-structured tests

Good work, Well-structured tests
MantashaNoyela (Migrated from github.com) reviewed 2025-02-25 22:21:05 -08:00
app/index.tsx
MantashaNoyela (Migrated from github.com) commented 2025-02-25 22:21:05 -08:00
Copy Link

I have changed it

I have changed it
djwesty (Migrated from github.com) reviewed 2025-02-25 23:21:19 -08:00
package.json
djwesty (Migrated from github.com) commented 2025-02-25 23:21:19 -08:00
Copy Link

No, the code change. adding the carret

No, the code change. adding the carret
djwesty (Migrated from github.com) reviewed 2025-02-25 23:21:48 -08:00
components/CalculatorState.tsx
@ -0,0 +28,4 @@
} catch (error) {
return null;
}
};
djwesty (Migrated from github.com) commented 2025-02-25 23:21:48 -08:00
Copy Link

No, just address changes with a new commit

No, just address changes with a new commit
djwesty (Migrated from github.com) approved these changes 2025-02-25 23:23:45 -08:00
djwesty (Migrated from github.com) left a comment
Copy Link

Approved for when all conversations resolved

Approved for when all conversations resolved
Sign in to join this conversation.
Reviewers
No reviewers
djwesty
Labels
Clear labels   
Large

5-10 Days

  
Medium

2-5 Days

  
Small

0-2 Days

  
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
wontfix

This will not be worked on

No Label
Large
Medium
Small
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No Milestone
Project Sprint 2
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
1 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: david/poker-chips-helper#37
No description provided.
Delete Branch "MantashaNoyela/26"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?