Skip to content
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

Moved @YoshiMan 's copy buttons inside the line Edit #222

Merged
merged 1 commit into from Nov 18, 2016

Conversation

@jounathaen
Copy link
Member

@jounathaen jounathaen commented Nov 17, 2016

Think this looks a bit more elegant (at least on Cinnamon)
qtpass1

@jounathaen
Copy link
Member Author

@jounathaen jounathaen commented Nov 17, 2016

I can't test these layout changes on other window toolkits, so i'd be very interested, how this looks on Windows, Mac, KDE, etc.
Screenshots would be great 👍

@annejan
Copy link
Member

@annejan annejan commented Nov 17, 2016

macOS screen shot 2016-11-17 at 20 06 26

The copy button would probably look nicer on the right side of the field . . but this is already a lot nicer than it was . .

I'll try on KDE and Windows later tonight.. Weird that Cinnamon doesn't use/supply a theme.
(you seem to be getting the fallback icons for buttons like Windows and macOS do)

@YoshiMan
Copy link

@YoshiMan YoshiMan commented Nov 17, 2016

My Screenshot from Antergos Linux (4.8.8-1) and Gnome (3.22.0-1)

antergoslinuxgnome

@annejan
Copy link
Member

@annejan annejan commented Nov 18, 2016

I think I'll merge this (in the morning) and have a look at swapping the copy button to the right.

@YoshiMan
Copy link

@YoshiMan YoshiMan commented Nov 18, 2016

i think its not a good idea to move the button to the right, because you select a password on the left panel. If you place the button on the right you have to move the mouse across the whole screen.

@annejan annejan merged commit 69f92b8 into IJHack:master Nov 18, 2016
3 of 4 checks passed
3 of 4 checks passed
Snap CI The Snap CI build failed on Nov 17, 2016!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
linthub Linthub has cleared the pull request, no code style suggestions found.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants