Skip to content

Conversation

@ice9js
Copy link
Contributor

@ice9js ice9js commented Mar 2, 2023

Proposed changes:

This patch adds a 'view switch' which will allow for switching between the classic WP Admin view for the feedback area and the new Jetpack Forms dashboard once it becomes enabled.

The switch is similar to the one already implemented for switching between WP Admin and Calypso views but it is also quite different in that both views are actually part of WP Admin itself.

Choosing an option through the dropdown, will also set that as the preferred option for the future.

Screenshot 2023-03-02 at 11 31 04

Other information:

  • Have you written new tests for your changes, if applicable?
  • Have you checked the E2E test CI results, and verified that your changes do not break them?
  • Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

pcsBup-Pl-p2

Does this pull request change what data or activity we track or use?

No.

Testing instructions:

Enable the new dashboard and the switch with these filters:

add_filter( 'jetpack_contact_form_use_package', '__return_true' );
add_filter( 'jetpack_forms_dashboard_enable', '__return_true' );
  • Go to WP Admin and click Feedback in the sidebar, you should be redirected to the new dashboard which currently also saves as the default setting.
  • There should be a View dropdown in the upper right corner of the screen.
  • Select Classic from the dropdown.
  • You should now be on the old WP Admin view.
  • If you go to any other WP Admin screen and click back on 'Feedback', it should now automatically show you the WP Admin interface which was your last preferred option.
  • Switch back to the Modern version.
  • Verify that Modern is now the default again.
  • You should be able to access either view manually using the URLs: admin.php?page=jetpack-forms or edit.php?post_type=feedback
  • Accessing a view through the URL without the preferred-view param should not affect your default setting.

@ice9js ice9js added the [Status] Needs Team Review Obsolete. Use Needs Review instead. label Mar 2, 2023
@ice9js ice9js self-assigned this Mar 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

Are you an Automattician? You can now test your Pull Request on WordPress.com. On your sandbox, run

bin/jetpack-downloader test jetpack add/jetpack-forms-dashboard-view-switch

to get started. More details: p9dueE-5Nn-p2

@github-actions
Copy link
Contributor

github-actions bot commented Mar 2, 2023

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

  • ✅ Include a description of your PR changes.
  • ✅ All commits were linted before commit.
  • ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  • ✅ Add testing instructions.
  • ✅ Specify whether this PR includes any changes to data or privacy.
  • ✅ Add changelog entries to affected projects

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖


The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.


Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped.
Then, add the "[Status] Needs Team review" label and ask someone from your team review the code.
Once you’ve done so, switch to the "[Status] Needs Review" label; someone from Jetpack Crew will then review this PR and merge it to be included in the next Jetpack release.


Jetpack plugin:

  • Next scheduled release: April 4, 2023.
  • Scheduled code freeze: March 28, 2023.

Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

The switch is similar to the one already implemented for switching between WP Admin and Calypso views but it is also quite different in that both views are actually part of WP Admin itself.

It works for me locally, but not on WoA. Do the 2 views conflict with each other?

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. and removed [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Mar 2, 2023
@ice9js
Copy link
Contributor Author

ice9js commented Mar 2, 2023

It seems the issue is WP.com is using a separate entrypoint for the contact form module, which we never updated to use the package. I'd say that's probably a separate issue that needs addressing on it's own, especially in the context of #28791, but not sure if it's that relevant to this PR.

@ice9js ice9js removed the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Mar 2, 2023
@ice9js
Copy link
Contributor Author

ice9js commented Mar 2, 2023

On some further investigation, it seems that once WP.com does render the dashboard, there are indeed some conflicts between this switch and existing WP.com menu implementation. I'll need to add a few more changes to get this to work reliably in both environments.

@ice9js
Copy link
Contributor Author

ice9js commented Mar 2, 2023

In the meantime, I'm hoping D103406-code will help sort things out on WP.com @jeherve.

@github-actions github-actions bot added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Mar 2, 2023
@ice9js ice9js added [Status] Needs Team Review Obsolete. Use Needs Review instead. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 3, 2023
CGastrell
CGastrell previously approved these changes Mar 3, 2023
Copy link
Contributor

@CGastrell CGastrell left a comment

Choose a reason for hiding this comment

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

"View" dropdown shows up when filter is enabled. Switching worked fine on its own. We need to give that app some more love though

@CGastrell CGastrell added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Team Review Obsolete. Use Needs Review instead. labels Mar 3, 2023
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

The buttons now work, but something seems off with the focus:

image

@jeherve jeherve added [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it and removed [Status] Needs Review This PR is ready for review. labels Mar 6, 2023
@jeherve jeherve added the [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. label Mar 6, 2023
@ice9js
Copy link
Contributor Author

ice9js commented Mar 7, 2023

Looks like there were some conflicting class names on .com, I think the latest changes should resolve that.

@ice9js ice9js added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 7, 2023
jeherve
jeherve previously approved these changes Mar 8, 2023
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This tests well for me now.

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 8, 2023
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Mar 8, 2023
@jeherve jeherve enabled auto-merge (squash) March 8, 2023 18:31
@jeherve jeherve merged commit d0396cf into trunk Mar 8, 2023
@jeherve jeherve deleted the add/jetpack-forms-dashboard-view-switch branch March 8, 2023 18:57
@github-actions github-actions bot added this to the jetpack/12.0 milestone Mar 8, 2023
@github-actions github-actions bot removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Forms [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants