-
Notifications
You must be signed in to change notification settings - Fork 17
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
Account Settings Cleanup - Support / FAQ / Email (2U) #198
Account Settings Cleanup - Support / FAQ / Email (2U) #198
Conversation
# Conflicts: # Core/Core/Configuration/Config/Config.swift
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.
- It would be nice if you could add a loader while loading the URLs of Privacy, TOS, etc.
} | ||
|
||
private func completePath(url: String) -> String { | ||
let langCode = String(Locale.preferredLanguages.first?.prefix(2) ?? "") |
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.
Hardcoding the prefix isn't a good option. I guess you can use the following code to get the desired code
let langCode: String
if #available(iOS 16, *) {
langCode = Locale.current.language.languageCode?.identifier ?? ""
} else {
langCode = Locale.current.languageCode ?? ""
}
if let supportedLanguages = supportedLanguages, | ||
!supportedLanguages.contains(langCode) { | ||
return url | ||
} | ||
|
||
let URL = URL(string: url) | ||
let host = URL?.host ?? "" | ||
let components = url.components(separatedBy: host) | ||
|
||
if components.count != 2 { | ||
return url | ||
} | ||
|
||
if let firstComponent = components.first, let lastComponent = components.last { | ||
return "\(firstComponent)\(host)/\(langCode)\(lastComponent)" | ||
} | ||
|
||
return url | ||
} |
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.
Please correct the indentation.
XCTAssertEqual(config.agreement.cookiePolicyURL, URL(string: "https://www.example.com/cookie")) | ||
XCTAssertEqual(config.agreement.dataSellContentURL, URL(string: "https://www.example.com/sell")) | ||
XCTAssertEqual(config.agreement.supportedLanguages, ["es"]) |
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.
How about moving the AgreementConfig tests to a different file, like to AgreementConfigTests? Thoughts?
private func button(linkViewModel: LinkViewModel, isEmailSupport: Bool = false) -> some View { | ||
Button { | ||
if isEmailSupport { viewModel.trackEmailSupportClicked() } | ||
UIApplication.shared.open(linkViewModel.url) |
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.
Before opening the URL can you please check whether the link is openable or not? If not it would be nice if you could show an error/info message.
private func terms(url: URL) -> some View { | ||
navigationLink( | ||
viewModel: .init( | ||
url: url, | ||
title: ProfileLocalization.terms | ||
) | ||
) | ||
} | ||
|
||
private func privacy(url: URL) -> some View { | ||
navigationLink( | ||
viewModel: .init( | ||
url: url, | ||
title: ProfileLocalization.privacy | ||
) | ||
) | ||
} | ||
|
||
private func cookiePolicy(url: URL) -> some View { | ||
navigationLink( | ||
viewModel: .init( | ||
url: url, | ||
title: ProfileLocalization.cookiePolicy | ||
) | ||
) | ||
} | ||
|
||
private func dataSellContent(url: URL) -> some View { | ||
navigationLink( | ||
viewModel: .init( | ||
url: url, | ||
title: ProfileLocalization.doNotSellInformation | ||
) | ||
) | ||
} |
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.
All these functions are doing the same thing; I guess you can pass the title as parameter as well with the URL.
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.
This methods for mapping -> viewModel.contactSupport().map(supportInfo)
) | ||
} | ||
|
||
private func faq(url: URL) -> some View { |
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.
I guess supportInfo(....
and faq( ....
can also use a common named method. Thoughts?
public static let cookiePolicy = ProfileLocalization.tr("Localizable", "COOKIE_POLICY", fallback: "Cookie Policy") | ||
/// Do Not Sell my Personal Information | ||
public static let doNotSellInformation = ProfileLocalization.tr("Localizable", "DO_NOT_SELL_INFORMATION", fallback: "Do Not Sell my Personal Information") |
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.
I'm not sure but you might need to follow the screen pattern for labels. So you need to use only the first character in caps in the whole text.
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.
@saeedbashir we are trying to follow current app as mentioned here #117
Do we need to change this string @moiz994 ?
Thank you
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.
@rnr we have discussed and I changed
@rnr This was an inconsistency in the prod app so would be good if we fixed it now. Thank you. |
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 👍
* chore: add faq, open in browser screen * chore: merge develop to branch * chore: add new links Cookie Policy, Do Not Sell my Personal Information and refactor * chore: add folder subviews to profile * chore: add support language to support urls * chore: add create button * chore: add tests * chore: fix language local * chore: changes PR feedback * chore: change strings * chore: change strings * chore: changes from PR feedback * chore: add progress to web browser * chore: show always progress when web page loading * chore: add show progress param to web browser
Support info - Contact support #118
FAQ #119
Support info - Terms of Service / Privacy Policies #120
Open Account links inside app
Configuration flags:
FEEDBACK_EMAIL_ADDRESS: '[email protected]'
FAQ_URL: 'link'
AGREEMENT_URLS:
PRIVACY_POLICY_URL: 'link'
TOS_URL: 'link'
COOKIE_POLICY_URL: 'link'
DATA_SELL_CONSENT_URL: 'link'