-
Notifications
You must be signed in to change notification settings - Fork 836
Forms: move componentized view actions to modal header on mobile #45410
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
base: trunk
Are you sure you want to change the base?
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! |
Code Coverage SummaryCoverage changed in 3 files.
2 files are newly checked for coverage.
Full summary · PHP report · JS report If appropriate, add one of these labels to override the failing coverage check:
Covered by non-unit tests
|
4adcd80
to
3e24ae4
Compare
3e24ae4
to
6038858
Compare
yeah, I have the same thoughts about the icons. We need to provide tappable touch target. For example, each icon can be 24x24 px, but it should sit inside a button or area that is at least 48x48 px. and there should be at least 8px spacing between adjacent tap targets to avoid accidental touches.
Given my points above, I'd lean towards this approach. |
This PR is not quite ready. It changes the wrong modal. So you end up seeing the one you resize but not actual one. |
I looked into this and the Modal that you see is created by a dataView action. currently they don't support header props such as headerActions. See https://developer.wordpress.org/block-editor/reference-guides/packages/packages-dataviews/#rendermodal. I think the approach to take here would be to add our own custom header or add support for such action on the action model. |
…rap InboxResponse with actions and navigation
9a608c8
to
38ba1af
Compare
…atus swaps and read toggles properly
Follow-up to:
Proposed changes:
Before
After
Screen.Recording.2025-10-08.at.15.50.14.mov
Note that in desktop view we have "mark as unread/read" action, but in mobile view we don't have a good indicator showing what changed so for now I'm leaving those out:
Other information:
Jetpack product discussion
Does this pull request change what data or activity we track or use?
Testing instructions: