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

Add link to app password configuration #9755

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Conversation

fmoc
Copy link
Contributor

@fmoc fmoc commented Jun 1, 2022

Fixes #9754.

The first commit fixes a minor issue introduced in #9746.

screenshot_2022-06-01_10-03-37

@fmoc fmoc added this to the 3.0 milestone Jun 1, 2022
@fmoc fmoc requested a review from a team June 1, 2022 07:59
@fmoc
Copy link
Contributor Author

fmoc commented Jun 1, 2022

Regarding the screenshot, maybe we should move the new label above the Connecting to ... one? This way, the position of the latter would remain the same as on the OAuth2 credentials page.

@TheOneRing
Copy link
Member

Regarding the screenshot, maybe we should move the new label above the Connecting to ... one? This way, the position of the latter would remain the same as on the OAuth2 credentials page.

maybe we could display the hint directly below the password line edit?

@fmoc
Copy link
Contributor Author

fmoc commented Jun 1, 2022

screenshot_2022-06-01_17-44-14

Alternatively, this may work.

screenshot_2022-06-01_17-58-33

I'll push the first one for now.

Useful for connecting to oC 10 servers which use basic auth.
@fmoc fmoc force-pushed the work/wizard-app-passwords branch from 967a046 to 4759586 Compare June 1, 2022 17:10
@tbsbdr
Copy link

tbsbdr commented Jun 3, 2022

1st approach is better. I can provide more nitty-gritty refinements next week.

@dragotin
Copy link
Contributor

dragotin commented Jun 3, 2022

Hm, I fear people mix up with the "Forgot my password" link and click-without-reading.

Also - users will not know what an "app password" exactly is. Any link to help?

@TheOneRing
Copy link
Member

Sadly branded clients don't provide doc and until now we don't have a solution for that.

@fmoc
Copy link
Contributor Author

fmoc commented Jun 15, 2022

Just had a quick conversation with @tbsbdr discussing this problem.

TL;DR: the text should be moved above the username/password form, below the "Enter your credentials" label.

Some background: the old wizard placed it right above the form as well:

screenshot_2022-06-15_16-18-07

There are two good reasons to move the app password link label into the proposed position:

a) existing users are used to finding the link above the form
b) typically, "forgot password" links are just below the password line edit on most websites and in many apps (@dragotin also suggested this in #9755 (comment))

screenshot_2022-06-15_16-22-50

@tbsbdr
Copy link

tbsbdr commented Jun 15, 2022

maybe put it even more "out of scope"?
but having it above the username+pw fields should prevent confusion with a pw-reset 👍
171449209-d5664725-9597-4eb0-812a-f3fbf57b2a0f

@fmoc
Copy link
Contributor Author

fmoc commented Jun 15, 2022

I'll push my original proposal for now so it can be tested. I'd like to hear @TheOneRing and @dragotin's opinion.

default:
Q_UNREACHABLE();
}
const QString usernameLabelText = []() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enumToStirng

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be easier to just add a helper to the theme?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, how to handle translations?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes in the theme.
QCoreApplication::translate("enumName","foo")

src/libsync/theme.cpp Show resolved Hide resolved
@fmoc fmoc force-pushed the work/wizard-app-passwords branch from 747f9d0 to 8283edb Compare June 17, 2022 16:20
@fmoc fmoc requested a review from TheOneRing June 17, 2022 16:22
@sonarcloud
Copy link

sonarcloud bot commented Jun 17, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@TheOneRing TheOneRing merged commit 040b185 into master Jun 21, 2022
@delete-merged-branch delete-merged-branch bot deleted the work/wizard-app-passwords branch June 21, 2022 09:11
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.

Inform about app passwords in new wizard
4 participants