Feature/2888 discussion settings#3387
Merged
kwonye merged 57 commits intofeature/site-settings-reviewfrom Nov 24, 2015
Merged
Conversation
…ion-settings Conflicts: WordPress/src/main/java/org/wordpress/android/models/SiteSettingsModel.java WordPress/src/main/java/org/wordpress/android/ui/prefs/DotComSiteSettings.java WordPress/src/main/java/org/wordpress/android/ui/prefs/SiteSettingsFragment.java WordPress/src/main/res/xml/site_settings.xml
…ion-settings Conflicts: WordPress/src/main/java/org/wordpress/android/ui/prefs/notifications/NotificationsSettingsFragment.java
Contributor
There was a problem hiding this comment.
typo: "You can override thee settings"
Contributor
|
I noticed that there is no consistency to capitalization. There's "Allow Comments" and below there is "Links in comments". Can you do an audit of all the newly committed text in |
Contributor
|
Crash. I changed a discussion setting, backed out of settings, and was navigating around the app. (on a .com site) |
Contributor
|
Unchecking "Allow Comments" still allows comments on dubkwon.wordpress.com |
Contributor
Author
I'm going to have @mattmiklic take a look through the design once this is merged. There will be a cleanup PR to address issues like this. |
Contributor
Author
Addressed in a65770f |
Contributor
Turns out I was unclear about the setting. "Allow comments" only applies to new posts. @tonyr59h has made a note to address the confusion in the polish PR, so effectively addressed. |
Contributor
|
Another crash (on self-hosted site): |
Contributor
|
On self-hosted, setting |
Member
I spoke with @drw158 about this while designing this section and he noted that the Material Design guidelines suggest sentence case for all labels:
So let's standardize on this. |
Contributor
Author
Contributor
|
Added #3434 as it is required for settings to go into |
Contributor
|
Crash when accessing settings while in airplane mode: |
Contributor
|
Great job! Not able to break it anymore 😛
|
kwonye
added a commit
that referenced
this pull request
Nov 24, 2015
…n-settings Feature/2888 discussion settings
24 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
All new settings complete and some old ones re-visited for self-hosted sites. In order to test most of the self-hosted settings you'll have to update one of the files on your WordPress box. PM me for details.
cc @kwonye