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

Export: Automatically fetch advanced settings when switching to exporter #3158

Merged
merged 1 commit into from
Feb 9, 2016

Conversation

jordwest
Copy link
Contributor

@jordwest jordwest commented Feb 8, 2016

This PR triggers a fetch of the export advanced settings when:

  • The user switches to the export page
  • The user changes sites while viewing the export page

Automated Testing

(cd client/state/site-settings/exporter && make test)

Browser Testing

  1. Open the Redux developer console, then go to a site's Settings > Export page.
  2. Check that the EXPORT_ADVANCED_SETTINGS_FETCH and EXPORT_ADVANCED_SETTINGS_RECEIVE actions have fired in the Redux developer console:
    screen_shot_2016-02-08_at_6_11_03_pm
  3. Pop open the advanced export settings by clicking the down arrow beside the Export All button.
  4. Keep an eye on the dropdowns as you switch to another site.
  5. The dropdowns should be replaced with pulsing placeholders while the settings for that site are fetched:
    screen shot 2016-02-09 at 7 37 26 am
  6. The dropdowns should then reappear. Note that they are not yet populated in this PR.
  7. Switch to the original site.
  8. This time, the placeholders should not appear - the previous fetched settings are loaded immediately from the store. You should however see a new EXPORT_ADVANCED_SETTINGS_FETCH action fire - this is to refresh the settings in the background to ensure they are up to date.

@jordwest jordwest self-assigned this Feb 8, 2016
@jordwest jordwest added this to the Export: First Release milestone Feb 8, 2016
@jordwest jordwest mentioned this pull request Feb 8, 2016
11 tasks
@jordwest jordwest added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. and removed [Status] In Progress labels Feb 8, 2016
const siteId = state.ui.selectedSiteId;
return {
siteId: siteId,
shouldShowPlaceholders: !advancedSettings( state, siteId )
Copy link
Member

Choose a reason for hiding this comment

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

Very minor thing: Here and in one or two spots later, we're missing a space after the !.

@dllh
Copy link
Member

dllh commented Feb 9, 2016

This works well for me. I suggested a minor fix, but otherwise, I think we should squash commits as needed and 🚢.

Squashed:
Export: Show placeholders while loading options
Export: Make existing tests pass
Export: Add unit tests for fetchingAdvancedSettings reducer
Export: Remove redundant serialize/deserialize
@jordwest jordwest force-pushed the add/exporter/auto-fetch-settings branch from f7d774c to ad876ca Compare February 9, 2016 11:00
@jordwest jordwest removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Feb 9, 2016
jordwest added a commit that referenced this pull request Feb 9, 2016
…ings

Export: Automatically fetch advanced settings when switching to exporter
@jordwest jordwest merged commit 569be95 into master Feb 9, 2016
@jordwest jordwest deleted the add/exporter/auto-fetch-settings branch February 9, 2016 11:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants