search

jazzband / django-two-factor-auth

Complete Two-Factor Authentication for Django providing the easiest integration into most Django projects.
MIT License
1.71k stars 448 forks source link

Add white background to QR image for browsers in dark mode #503

Closed jcu721 closed 2 years ago

jcu721 commented 2 years ago

My computer preference theme is set to Dark. Django interprets this nicely, but the QR code shows up as dark on dark and is very difficult to see. Can be read by some QR readers but not by others.

Expected Behavior

The QR image should always have a white background instead of transparent. Browsers with dark mode enabled that are overriding the base template should be able to have a readable QR image that can be scanned easily.

Current Behavior

When dark mode is enabled, the background is set to black and the transparent image shows up directly on the default black background. It can be read by one phone I tested with, but not the 1Password browser QR scanner. It is also very difficult to even visually tell there is a QR code there.

Screen Shot 2022-05-16 at 6 26 25 PM

Possible Solution

Hardcode the style background to be white on the img tag. <p><img src="{{ QR_URL }}" alt="QR Code" style="background:white"/></p>

This results in an easy to see QR image with a nice margin/border:

Screen Shot 2022-05-16 at 6 28 16 PM

Steps to Reproduce (for bugs)

  1. Have a custom _base.html template that extends the base django template: {% extends "admin/base.html" %}
  2. Have computer (mac, in my case) theme set to dark (System Preferences -> General -> Appearance)
  3. Navigate through 2FA setup to get to QR page
  4. Try to scan with in browser QR scanner

Context

The dark on dark QR code can actually be read by a phone (tested with iphone 13 mini), but cannot be read by the 1Password in-browser QR scanner. I'd imagine it would also work poorly with older phones / certain cameras.

Your Environment

claudep commented 2 years ago

Could you explore adding bg-white/bg-light Bootstrap classes instead of direct style?

claudep commented 2 years ago

@jcu721, do you plan to work on a patch?

jcu721 commented 2 years ago

@claudep just opened a PR, https://github.com/jazzband/django-two-factor-auth/pull/504, let me know if I need to do any further testing.