-
-
Notifications
You must be signed in to change notification settings - Fork 774
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
Update Done/cancel buttons #5168
base: 7.7.0
Are you sure you want to change the base?
Conversation
f50cac1
to
6541f52
Compare
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.
Thanks, these mostly look good, found one regression (commented on appropriate file). I also found some that weren't updated:
For the two factor and change temp, I had to fudge some code to even enter that screen.
I also found one double Done button situation that was missed, we should ask design for instruction here:
Settings > My Languages > Add another language so you have multiple > See Edit button, tap it:
Lastly, there's a series of other pieces of feedback, but they seem a bit more involved due to changing existing X buttons into a label. Maybe we can break that off into a separate task? I would like to prioritize them separately at least, for the chance that they will be deprioritized to a later release.
Article Gallery
Article TOC
Article References
Editor Find in Page
@@ -181,7 +181,7 @@ class SinglePageWebViewController: ThemeableViewController, WMFNavigationBarConf | |||
let titleConfig = WMFNavigationBarTitleConfig(title: "", customView: nil, alignment: .hidden) | |||
|
|||
if navigationController?.viewControllers.first === self { | |||
closeConfig = WMFNavigationBarCloseButtonConfig(text: CommonStrings.doneTitle, target: self, action: #selector(closeButtonTapped(_:)), alignment: .leading) | |||
closeConfig = WMFNavigationBarCloseButtonConfig(text: CommonStrings.cancelActionTitle, target: self, action: #selector(closeButtonTapped(_:)), alignment: .leading) |
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.
Phabricator: https://phabricator.wikimedia.org/T383460
Notes
Test Steps