Credentials dropdown in sign in window#4693
Conversation
|
Lightbeam link |
allister-beamable
left a comment
allister-beamable
left a comment
There was a problem hiding this comment.
Some style/syntax comments, but nothing really demanding changes. Otherwise looks good!
Any concerns about what happens when there is an egregiously large set of saved credentials? I suppose if the main symptom is that the rendered list threatens to overflow the available space, the people adding credentials will notice and perhaps avoid adding more. 😸
I would like to hear from @cdhanna and @DiasAtBeamable on this. Feels like we can take our time with it since it is not harming anything not to have it.
| { | ||
| var badgeColor = new Color(0,0,0,.3f); | ||
| if (_editorAPI?.BeamCli.CurrentRealm?.IsProduction ?? false) | ||
| if (!_editorAPI?.BeamCli?.IsLoggedOut ?? true) |
There was a problem hiding this comment.
Ugh, I lowkey hate this syntax. I suppose it is necessary because the value of _editorAPI?.BeamCli?.IsLoggedOut ends up being null if any of the question-marked ones is null?
No need to change it, just grumping about C# syntax.
| } | ||
| set | ||
| { | ||
| var vv = JsonUtility.ToJson(new SavedCredentialsArray{items = value |
There was a problem hiding this comment.
Not thrilled about vv as a variable name; I suppose its only use is right next to it, so it is not actually all that unclear, and value is a keyword. Were it my code, I would likely name it credsValue or something. Verbosity in hopes of clarity.
No real need to change, just a statement of code style preference: I would rather see longer, more descriptive variable names.
| try | ||
| { | ||
| return JsonUtility.FromJson<SavedCredentialsArray>( | ||
| EditorPrefs.GetString("Beamable.SavedCredentials", "{}")).items.ToList(); |
There was a problem hiding this comment.
Unless we have all the tooling for clearing out the editor prefs- I am sad about mixing our data stores.
Is the goal to remember credentials from between all your projects, or just this one Unity project? If it is just the one unity project, I would recommend we keep our own json file in the .beamable folder. My thinking there is; from the users' perspective, there should be one place where persistent stuff is stored.
There was a problem hiding this comment.
Oh, I forgot that the EditorPrefs is shared between projects. I am not so confident in using .beamable for that though, what do You thinkg about using EditorUserSettings GetConfigValue and SetConfigValue? Then it would be stored in the %PROJECT_ROOT%/UserSettings/EditorUserSettings.asset which is fine for me, since I decided not to store the password there.
| } | ||
| } | ||
|
|
||
| EditorUtility.DisplayCustomMenu( |
There was a problem hiding this comment.
if possible, I would vote we re-use the existing beam editor dropdown window stuff we use in other parts of the editor; rather than use the right-click visual style. I think we could control the UI a little better to even group accounts by their alias and such.
There was a problem hiding this comment.
I went this way because the IMGUI with dropdown had issues with depth sorting, but I am open for other approach, more consistent with the rest of the UI
|
Lightbeam link |
DiasAtBeamable
left a comment
DiasAtBeamable
left a comment
There was a problem hiding this comment.
LGTM, I've just added a suggestion.
| { | ||
| try | ||
| { | ||
| var savedValue = EditorUserSettings.GetConfigValue("Beamable.SavedCredentials"); |
There was a problem hiding this comment.
Would be nice to have "Beamable.SavedCredentials" as a const string variable.
4 participants
Adds dropdown menu with saved credentials to choose from.