VideoPress: Replace custom Checkbox with CheckboxControl, fix invisible filter/privacy checkboxes#48175
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. Boost plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. Protect plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
The Video Privacy toggle at the bottom of the VideoPress dashboard was using the local CheckboxCheckmark wrapper around a custom-styled checkbox that relied on a ::before pseudo-element for its visible box but never defined background/border, leaving the control blank on top of any cascade that didn't paint a default. Replace it with WP's @wordpress/components CheckboxControl for native rendering and accessibility, and move the component-local layout rule (margin-top for spacing under the Settings heading) into its own style.module.scss alongside the consumer. Also routes the existing disabledReason copy through CheckboxControl's `help` prop.
…n UI Now that the site-settings privacy toggle uses @wordpress/components CheckboxControl directly, the local Checkbox component is only used by: - video-filter's CheckboxCheckmark (filter panel on the library page) - video-list / video-row "select all" + per-row selection checkboxes, both permanently gated behind a hardcoded `showCheckbox = false` with a `// TODO: implement bulk actions` note — never rendered in the current UI Migrate CheckboxCheckmark to CheckboxControl and delete the list/row selection plumbing entirely, then remove the components/checkbox/ directory (component, styles, stories, tests, types). video-filter: - CheckboxCheckmark now renders CheckboxControl with a .filter-checkbox class that adds `padding-top: var(--spacing-base)` for vertical rhythm between filter options. - Drop the `for` and `disabledReason` props plus the internal DisabledReasonTooltip — neither was actually used by any call site. - Remove now-dead `.filters-section .checkbox-container:last-child` and `.title-adornment` rules. video-list: - Remove `selected` / `setSelected` state, `allSelected`, `handleAll`, and the `showCheckbox = false` constant from both VideoList and LocalVideoList. - Drop the select-all header checkbox and stop passing `checked` / `onSelect` / `showCheckbox` through to the row components. video-row: - Drop the CheckboxControl render block + wrapper div. - Drop `checked`, `onSelect`, `showCheckbox` props from the signature and the VideoRowProps type. - Drop the row-level keyboard/click handling that only existed to toggle the (never-rendered) checkbox: `role=button`, `tabIndex`, `aria-label`, `onKeyDown`, `onKeyUp`, `keyPressed` state, `handleKeyDown`/`handleKeyUp`/`handleClick`/`isSpaceOrEnter`. - Simplify `handleInfoWrapperClick` to only toggle mobile expansion. - Remove the orphan `.checkbox-wrapper-small` SCSS rule (`.pressed` stays — still used by video-row/error.tsx). Net: ~377 lines of dead code gone, no visible UI change.
6a52b8c to
e0e2237
Compare
Code Coverage SummaryCoverage changed in 3 files.
Full summary · PHP report · JS report Coverage check overridden by
Coverage tests to be added later
|
Part of #48160
Proposed changes
The Video Privacy toggle at the bottom of the VideoPress dashboard, and the checkboxes in the library's filter panel (Uploader / Privacy / Rating), were rendering as invisible boxes — labels present, no box. Root cause was the local custom Checkbox component doing
all: unsetoninput[type="checkbox"]and then relying on a::beforepseudo-element that only set dimensions, no background/border. Replacing it with@wordpress/components'CheckboxControlgives us native rendering for free.site-settings-section): swapped toCheckboxControl,disabledReasonrouted through the nativehelpprop, component-local layout rule moved into its ownstyle.module.scssnext to the consumer.video-filter):CheckboxCheckmarknow rendersCheckboxControland applies a.filter-checkboxclass withpadding-top: var(--spacing-base)for a comfortable vertical rhythm between options. Dropped theforanddisabledReasonprops (neither was used by any call site) and the unusedDisabledReasonTooltiphelper.video-list,video-row): the custom Checkbox's last two consumers were a "select all" header and per-row selection checkboxes, both permanently gated behindconst showCheckbox = false; // TODO: implement bulk actions— never rendered in the current UI. Migrated the would-be-rendered bits toCheckboxControland then removed the dead selection plumbing entirely:selected/setSelected/allSelected/handleAllstate, thechecked/onSelect/showCheckboxprops onVideoRow, the row-levelrole="button"/tabIndex/onKeyDown/onKeyUp/aria-labeland itskeyPressedstate/handlers (they only existed to toggle the never-rendered checkbox), and the orphan.checkbox-wrapper-smallSCSS. When bulk actions do ship, the implementer will useCheckboxControldirectly rather than resurrecting a custom component.components/checkbox/: with the three consumers migrated, the custom Checkbox component, its stylesheet, types, storybook story, and test are removed.Net: -392 / +15, no visible UI change except "previously blank checkboxes now render correctly."
Before


After:


Related product discussion/links
Does this pull request change what data or activity we track or use?
No.
Testing instructions
/wp-admin/admin.php?page=jetpack-videopress).Before/after screenshots below.