-
Notifications
You must be signed in to change notification settings - Fork 814
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
Forms: Add Feedback link to admin bar #42474
Conversation
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
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 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: The Jetpack plugin has different release cadences depending on the platform:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Hey @enejb 👋, This is a great and quick solution! 🙌 I wonder about the naming and whether it's clear enough, maybe something like 'Form Feedback' (adding Form) could help to clarify what this is (especially for new users)? It takes a bit more space tho. |
Code Coverage SummaryCoverage changed in 1 file.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
@crisbusquets @Copons any needs to coordinate this with anyone from Dotcom? Note that this Only applies to pages with forms on them. @enejb what happens if form is in footer/sidebar? It would be too much to render on every page. @ilonagl thoughts on what the link copy should be? |
@enejb how's the mobile version? |
If we are a single (page or post or any other post type) - we would show the link to the form responses. Even if it is a shortcode or a widget. In the current version we only link users to the default form responses page. No source attribute. But ideally we would send the user to the correct one.
The mobile version doesn't show the link at all. See |
Oh, hello Zap! ⚡ From what I understand, we want to give site admins a quick link to check replies for a specific form, right? Not sure how feasible that would be, but could we explore rendering the block differently so the link appears with the form? The |
Admin area felt more fitting semantically since that's where other admin stuff goes: also where you edit current page, or check stats for the current post. Also nice not to keep the page as WYSIWYG as possible with editor and no extra things appearing, potentially messing up with page layout. |
projects/packages/forms/src/contact-form/class-contact-form.php
Outdated
Show resolved
Hide resolved
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.
This is testing as advertised for me 👍
✅ Form responses link shows on individual posts and pages
✅ New link isn't displayed on index pages or mobile
I have a few minor questions and thoughts I've left as inline comments.
Edit: Looking at this screenshot again, the lack of an icon for "Form responses" stuck out. Are there plans for a follow-up addressing this?
projects/packages/forms/src/contact-form/class-contact-form.php
Outdated
Show resolved
Hide resolved
projects/packages/forms/src/contact-form/class-contact-form.php
Outdated
Show resolved
Hide resolved
I agree with @aaronrobertshaw that having an icon would help keep the links consistent with the rest. Can we reuse the existing ![]() |
… the edit page and edit post links
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.
This is getting closer, thanks for the iterations here @enejb 👍
I think we can clean up some of the icon styles, then it should be ready to go.
I've tested the current PR state along with my suggested tweak below and visually they are the same.
projects/packages/forms/src/contact-form/class-contact-form.php
Outdated
Show resolved
Hide resolved
Co-authored-by: Aaron Robertshaw <[email protected]>
Sorry I'm late. Did you implement tracking here? This way we'll be able to know how many people click that option 👀 |
This PR adds a quick link to the admin bar if the user is visiting a single page or post on the frontend and is someone that should have access to the dashboard.
It only shows up if we are also going to render the form.
On single page with a form on.
On a single page without a form.
To disable the newly added quick_link menu you can
do the following:
Proposed changes:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
No
Testing instructions: