Skip to content

Feature/3633 export site api - #3848

Merged
nbradbury merged 12 commits into
feature/delete-site-redesignedfrom
feature/3633-export-site-api
Mar 11, 2016
Merged

Feature/3633 export site api#3848
nbradbury merged 12 commits into
feature/delete-site-redesignedfrom
feature/3633-export-site-api

Conversation

@kwonye

@kwonye kwonye commented Mar 9, 2016

Copy link
Copy Markdown
Contributor

Fixes #3633

screenshot_20160309-141024
screenshot_20160309-141028

To test:

  1. Go to Site Settings and click "export content"
  2. Click "EXPORT CONTENT"
  3. An email will show up with your export zip

Needs review: @oguzkocer @nbradbury

nbradbury
nbradbury reviewed Mar 10, 2016
View reviewed changes
Comment thread WordPress/build.gradle Outdated
minSdkVersion 14
targetSdkVersion 23

vectorDrawables.useSupportLibrary = true

@nbradbury nbradbury Mar 10, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Was this change intentional? We haven't updated to v23.2 of the support library yet, so we're not ready to make this change.

@maxme maxme Mar 10, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yep revert this please - ref #3791

@kwonye kwonye Mar 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverting and moving addition of this line to #3856

@nbradbury nbradbury self-assigned this Mar 10, 2016
nbradbury
nbradbury reviewed Mar 10, 2016
View reviewed changes
Comment thread WordPress/src/main/java/org/wordpress/android/ui/prefs/SiteSettingsFragment.java Outdated
exportSite();
}
});
builder.setNegativeButton(R.string.cancel, new DialogInterface.OnClickListener() {

@nbradbury nbradbury Mar 10, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note that using builder.setNegativeButton(R.string.cancel, null); here will accomplish the same thing.

@kwonye kwonye Mar 11, 2016

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

🙇 thanks so much. You have no idea how irked I was I had to do all this just to close the dialog.

@nbradbury

nbradbury commented Mar 10, 2016

Copy link
Copy Markdown
Contributor

With the exception of the build.gradle change (which I think is why travis is failing), this looks good code-wise. But the text on the "Export your content" dialog could use some work. I realize it was copied from the web app, but here it doesn't really work.

  • It's too wordy. I recommend checking out the writing style page on Google Design.
  • "Keep in mind that this content can not be recovered in the future" makes sense on the web because export is part of the delete site process, but here it's separate - so that warning makes it sound like exporting might delete your content.
  • Similarly, "...that you can use in the future to resume where you left off" sort of makes sense on the web, but here it's just confusing.

I recommend something much simpler, like just "Your posts, pages, and settings will be emailed to you at %s."

@oguzkocer

oguzkocer commented Mar 10, 2016

Copy link
Copy Markdown
Contributor

I have reviewed the code as well, but nothing to add to Nick's comments. Looks good to me.

@kwonye

kwonye commented Mar 11, 2016

Copy link
Copy Markdown
Contributor Author

I recommend something much simpler, like just "Your posts, pages, and settings will be emailed to you at %s."

Addressed in df0f442

@nbradbury

nbradbury commented Mar 11, 2016

Copy link
Copy Markdown
Contributor

Looks good! :shipit:

nbradbury added a commit that referenced this pull request Mar 11, 2016
@nbradbury
Merge pull request #3848 from wordpress-mobile/feature/3633-export-si…
5b60e40 
…te-api

Feature/3633 export site api
@nbradbury
nbradbury merged commit 5b60e40 into feature/delete-site-redesigned Mar 11, 2016
@nbradbury
nbradbury deleted the feature/3633-export-site-api branch March 11, 2016 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nbradbury nbradbury

Labels

Delete Site

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@kwonye @nbradbury @oguzkocer @maxme