-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
Provide initial state for backupcodes in template #13481
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
Conversation
|
I also bumped the dependencies and added the compiled assets to the gitattributes |
ChristophWurst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a nitpick
will test in a minute, just to see how fast it loads :)
| el: '#twofactor-backupcodes-settings', | ||
| render: h => h(PersonalSettings) | ||
| }); | ||
| const initialStateElem = JSON.parse(atob(document.getElementById('twofactor-backupcodes-initial-state').value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: the variable name is misleading. I guess you wanted to do the .value below when you pass it to the store
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right fair enough...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why use atob?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because it is base64 encoded. Else we might run into some random errors with quotes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use json_encode directly for settings :)
https://github.com/nextcloud/server/blob/master/settings/templates/settings-vue.php#L30
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
still feels more error prone. base64 is guaranteed not to contain special chars that might break things
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right! Let's add this to #13488
09657ff to
53b7345
Compare
|
@skjnldsv can I also get a 👍 😉 |
53b7345 to
972f9e3
Compare
ChristophWurst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works nicely 👍
This saves a direct request to the server when loading the backup codes. There is no need for this as the data is already known. Signed-off-by: Roeland Jago Douma <[email protected]>
Signed-off-by: Roeland Jago Douma <[email protected]>
Signed-off-by: Roeland Jago Douma <[email protected]>
Signed-off-by: Roeland Jago Douma <[email protected]>
972f9e3 to
90f8687
Compare
This saves a direct request to the server when loading the backup codes.
There is no need for this as the data is already known.
Signed-off-by: Roeland Jago Douma [email protected]