Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add activeUser for default payer per group #16

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

ankitbahl
Copy link
Contributor

This change adds an active user option to group settings, which can be set per group. This is only saved in the devices localStorage, and therefore can be set per device.

Copy link

vercel bot commented Jan 3, 2024

@ankitbahl is attempting to deploy a commit to the Sebastien Castiel Team on Vercel.

A member of the Team first needs to authorize it.

@scastiel
Copy link
Member

scastiel commented Jan 8, 2024

Hi @ankitbahl, thanks a lot for the contribution, it looks great!

A couple of remarks about the PR:

  • Could you update it with the origin repo and resolve the conflicts (it’s nothing too bad 😉)
  • Could you make the local storage contain a participant ID instead of their name?
  • I think there is a tiny issue when I add a new participant to a group and want to select it as the active user (the option is New instead of the participant’s name)

Just tell me if you don’t have time (or don’t want) to make the changes, I will gladly take it from here if you prefer 😉

@scastiel scastiel linked an issue Jan 8, 2024 that may be closed by this pull request
@ankitbahl
Copy link
Contributor Author

Appreciate you taking a look ☺️ Great suggestions, I'll make those changes later today 🙂

@ankitbahl
Copy link
Contributor Author

Realized there was one more bug, should be good to look at now

@scastiel
Copy link
Member

scastiel commented Jan 9, 2024

Amazing, thanks @ankitbahl!

@scastiel scastiel merged commit 6bd3299 into spliit-app:main Jan 9, 2024
1 check failed
@ChristopherJohnston
Copy link
Contributor

Awesome addition!

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.

Tell the app who you are
3 participants