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

[SDK-681] Add account creation element #178

Merged
merged 4 commits into from
Dec 5, 2023

Conversation

rYuuk
Copy link
Contributor

@rYuuk rYuuk commented Dec 4, 2023

SDK-681

Description

  • Added account creation element scripts and prefab
  • Fixed wrong url in login with otp

How to Test

  • NA

Checklist

  • Tests written or updated for the changes.
  • Documentation is updated.
  • Changelog is updated.

@rYuuk rYuuk self-assigned this Dec 4, 2023
@rYuuk rYuuk requested a review from a team as a code owner December 4, 2023 14:47
@HarrisonHough
Copy link
Collaborator

A minor thing but the prefabs buttons has a bunch of empty events assigned.
image

Copy link
Collaborator

@HarrisonHough HarrisonHough left a comment

Choose a reason for hiding this comment

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

Looks good! some small things in the comments

@@ -24,27 +23,24 @@ private void OnEnable()
{
sendEmailButton.onClick.AddListener(OnSendEmailButton);
Copy link
Collaborator

@HarrisonHough HarrisonHough Dec 5, 2023

Choose a reason for hiding this comment

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

In theory we could make this component slightly more flexible by removing the auto assigning of the event listeners and instead make the appropriate functions public and assign them in the inspector. It would also mean that you might need the button references. But not a blocker for me if you think it's unnecessary to do that.

@rYuuk rYuuk merged commit 72b4cc3 into develop Dec 5, 2023
@rYuuk rYuuk deleted the feature/add-account-creation-element branch December 5, 2023 07:21
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