Skip to content

[1.x] Update two-factor-authentication-form.blade.php #414

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

Merged
merged 2 commits into from
Oct 30, 2020

Conversation

underwood
Copy link
Contributor

I had an issue where I couldn't scan the barcode when setting up 2FA. Determined the issue was using darkmode tailwind css styling. The Google authenticator app and VIP Access Authenticator don't pick up the barcode if it has a dark background right up against it. Add a p-4 white background resolved the issue on both apps. Propose adding this white border out of the box to prevent this issue if the user chooses a dark color adjacent to the barcode.

I had an issue where I couldn't scan the barcode when setting up 2FA. Determined the issue was using darkmode tailwind css styling. The Google authenticator app and VIP Access Authenticator don't pick up the barcode if it has a dark background right up against it. Add a p-4 white background resolved the issue on both apps. Propose adding this white border out of the box to prevent this issue if the user chooses a dark color adjacent to the barcode.
@bonzai
Copy link
Contributor

bonzai commented Oct 29, 2020

@underwood
Copy link
Contributor Author

underwood commented Oct 30, 2020

Can you also add similar styles to Inertia stack?
https://github.com/laravel/jetstream/blob/1.x/stubs/inertia/resources/js/Pages/Profile/TwoFactorAuthenticationForm.vue#L34

Good catch, I didn't think about the inertia side of things. Thanks.

#417

@driesvints driesvints changed the title Update two-factor-authentication-form.blade.php [1.x] Update two-factor-authentication-form.blade.php Oct 30, 2020
@taylorotwell
Copy link
Member

Can you show a screenshot of what it looks like with your code changes? The width looks good in responsive as well?

@underwood
Copy link
Contributor Author

Can you show a screenshot of what it looks like with your code changes? The width looks good in responsive as well?

see screenshots below. It looks good in mobile, tablet, desktop.

desktop
full
mobile
mobile

@taylorotwell
Copy link
Member

taylorotwell commented Oct 30, 2020

Does it make the QR code look oddly shifted left in non-dark mode? Should we add this padding background only with the dark variant?

@underwood
Copy link
Contributor Author

y, the QR code wouldn't quite line up with the text.

I'm using dark: modifiers for my dark mode so we could make these only for dark:

If someone would change their background to a dark background directly without using the dark modifier then they would run in to the issue of the QR code not working.

Here are some screenshots of how it looks in light mode.
light-desktop

light-mobile

@taylorotwell
Copy link
Member

If they were to change their background they could just modify the code directly. I'm not sure Jetstream should be made to account for every possible case. The code is your code after you install Jetstream. It is no longer Jetstream's code. You should feel free to modify it as much as you want or need for your application.

@underwood
Copy link
Contributor Author

Here's another option. Centering the QRcode would make it work either way.
#420

@taylorotwell
Copy link
Member

Can we just add the dark: modifiers here and on the Inertia PR?

@underwood
Copy link
Contributor Author

Can we just add the dark: modifiers here and on the Inertia PR?

done and done, this works, tested in light and dark mode, that lines the QR code up with the text in light mode.

@taylorotwell taylorotwell merged commit a933f4e into laravel:1.x Oct 30, 2020
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.

3 participants