-
Notifications
You must be signed in to change notification settings - Fork 42
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
Fixes #659 Adds a show password options for login #679
Conversation
221c633
to
6e39531
Compare
The ICONS in the default state looks horrible. Need tips from @ninavizz on how they should look like. We can then convert this into a complete widget which can be used in any other place too. |
You can update all the QLineEdit margins to 0px to get around this scaling issue: securedrop-client/securedrop_client/gui/widgets.py Lines 1314 to 1318 in 4d71556
Why is this happening? Looks like Qt doesn't respect CSS margins when rendering icons in QLineEdit (I believe this is where the Qt code would need to be modified in order to check for margins and scale appropriately: https://code.woboq.org/qt5/qtbase/src/widgets/widgets/qlineedit_p.cpp.html#352) I tested this by using their builtin clear textbox feature and you can see that it also looks large and pixelated when we use a QLineEdit margin that is non-zero: If you update the QLineEdit margins to 0px then our textboxes will be closer together in the form so I suggest adding a spacer in between the widgets. Also, if you want more control over the icon size, you will want to load a pixmap and scale it before adding it the icon. And for toggling, you might want to use our load_toggle_icon helper function. In the end, I wouldn't be surprised if you choose to go down the path of creating a custom QLineEdit widget that adds a toggle svg label or push button, because you'll have more control over the look and feel. |
6e39531
to
4f13dd3
Compare
@creviera I have added a new I have the svgs as provided by @ninavizz and when I tried to fix the aspect ratio issue, I broke it even more. I will learn from you how to fix this when you are online. |
a2fc708
to
97eed42
Compare
It's looking better! This is where @ninavizz provides image inventory: freedomofpress/securedrop-ux#17 I just want to note that while you wait for up-to-date icons from nina, you can still work on scaling the icon within the textbox. If you replace the eye_visible.svg with send.svg or any other svg, for instance, you'll notice that they all are resized to fit within a square. If the aspect ratio isn't a square, that is, 1:1, it'll look incorrect. So either, nina needs to provide you with a 1:1 version of the eye icons (which still might appear too small) or you'll need to scale the icons to keep the current aspect ratio of the image. You can see SvgPushButton for an example of how to do this or perhaps you'll come up with another way. |
The square size icons are fixing the issue. @creviera i tried with icons from another project of mine to verify (DEMO PURPOSE ONLY). Now, I will wait for @ninavizz :) |
@kushaldas Definitely an improvement—however the icons are too stylistically different from all the others. The "x" slash is also far too subtle in the difference between the two, and the lightness of the stroke(s) w/ the reflection on the iris make them too fussy. I'd prefer to keep with the ones I'd spec'd... what exactly do you need from me to "fix" the ones in the inventory? Are you using SVGs or PNGs? Do you basically need the same art, but with a white square behind it so that it registers a square aspect-ratio? |
Yes, I need square SVGs for this. |
Eye — Hidden
Eye — Visible
@kushal, if there's any problem with these icons... they're from here, and are actually in a square aspect-ratio (tho black) in their OG form. I just colored and sized them, and put them on a square the same color as the text fields, in mine (above). |
@ninavizz those are beautiful. Thank you :) |
It loads icons to show/hidden the password entry.
97eed42
to
4ebb106
Compare
Aaah, so lovely Kushal, thank u! :) |
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.
lgtm!
Description
Fixes #659
Test Plan
Checklist
If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable: