-
Notifications
You must be signed in to change notification settings - Fork 57
feat(rolling-content): hidden DataViews demo wizard #4728
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
Draft
thomasguillot
wants to merge
21
commits into
trunk
Choose a base branch
from
feat/rolling-content-demo
base: trunk
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
f411cd1
feat(rolling-content): scaffold hidden demo wizard with conditional menu
thomasguillot 5cdcce1
feat(rolling-content): wire up React routes with placeholder views
thomasguillot 137814b
feat(rolling-content): add types and fake dataset
thomasguillot 3c4e334
fix(rolling-content): use coprime multipliers for dataset variety
thomasguillot a359992
feat(rolling-content): all rolling content dataviews
thomasguillot 1632dcd
feat(rolling-content): add rolling content placeholder view
thomasguillot f32ae17
feat(rolling-content): edit and delete row actions with shared modals
thomasguillot 2328fb6
fix(rolling-content): defer __() to render and mark actions non-bulk
thomasguillot 9ede8ca
feat(rolling-content): add new entry info modal
thomasguillot f7e3d76
feat(rolling-content): manage entries modal with nested dataviews
thomasguillot 0f64a6f
fix(rolling-content): unify status pill and remove nested modal chrome
thomasguillot 49cfd4e
fix(rolling-content): drop inner manage-entries modal wrapper
thomasguillot b281a75
fix(rolling-content): render correct page id and inline view headers
thomasguillot 0fe4bdd
refactor(rolling-content): swap status pills for icons and plain text
thomasguillot 8468161
fix(rolling-content): restore Newspack chrome, inline actions, condit…
thomasguillot 69ac184
fix(rolling-content): set all view section to fullWidth
thomasguillot ba14ad1
fix(rolling-content): merge manage and add buttons into entries column
thomasguillot 673c5ff
fix(rolling-content): use $_GET directly and dedupe header text
thomasguillot e242b39
fix(rolling-content): anchor menu position so it's not buried at the …
thomasguillot 41f12d5
fix(rolling-content): force-highlight the active menu via parent_file…
thomasguillot abedb05
fix(rolling-content): address copilot review on modal state and sort …
thomasguillot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,209 @@ | ||
| <?php | ||
| /** | ||
| * Newspack Rolling Content Demo. | ||
| * | ||
| * A hidden, URL-gated admin area used to explore DataViews patterns with | ||
| * fake in-memory data. Not intended for production use. | ||
| * | ||
| * @package Newspack | ||
| */ | ||
|
|
||
| namespace Newspack; | ||
|
|
||
| defined( 'ABSPATH' ) || exit; | ||
|
|
||
| require_once NEWSPACK_ABSPATH . '/includes/wizards/class-wizard.php'; | ||
|
|
||
| /** | ||
| * Rolling Content demo wizard. | ||
| */ | ||
| class Rolling_Content_Demo extends Wizard { | ||
|
|
||
| /** | ||
| * The slug of the main page (All Rolling Content). | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $slug = 'newspack-rolling-content'; | ||
|
|
||
| /** | ||
| * The capability required to access this wizard. | ||
| * | ||
| * @var string | ||
| */ | ||
| protected $capability = 'manage_options'; | ||
|
|
||
| /** | ||
| * Hidden from Newspack's own submenu; this wizard manages its own top-level menu conditionally. | ||
| * | ||
| * @var bool | ||
| */ | ||
| protected $hidden = true; | ||
|
|
||
| /** | ||
| * Slug for the "Add Rolling Content" placeholder page. | ||
| */ | ||
| const SLUG_ADD = 'newspack-rolling-content-add'; | ||
|
|
||
| /** | ||
| * Get the name for this wizard. | ||
| * | ||
| * @return string | ||
| */ | ||
| public function get_name() { | ||
| return __( 'Rolling Content', 'newspack-plugin' ); | ||
| } | ||
|
|
||
| /** | ||
| * Override the parent slug check so both demo pages are recognized. | ||
| * | ||
| * @return bool | ||
| */ | ||
| public function is_wizard_page() { | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Recommended | ||
| $page = isset( $_GET['page'] ) ? sanitize_key( wp_unslash( $_GET['page'] ) ) : ''; | ||
| return in_array( $page, [ $this->slug, self::SLUG_ADD ], true ); | ||
| } | ||
|
|
||
| /** | ||
| * Render the container div. Override the parent so the id matches the current page slug, | ||
| * not `$this->slug`. Required because React mounts into `getElementById(pageParam)`. | ||
| */ | ||
| public function render_wizard() { | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Recommended | ||
| $page = isset( $_GET['page'] ) ? sanitize_key( wp_unslash( $_GET['page'] ) ) : ''; | ||
| $id = in_array( $page, [ $this->slug, self::SLUG_ADD ], true ) ? $page : $this->slug; | ||
| ?> | ||
| <div class="newspack-wizard <?php echo esc_attr( $id ); ?>" id="<?php echo esc_attr( $id ); ?>"></div> | ||
| <?php | ||
| } | ||
|
|
||
| /** | ||
| * Register admin pages. | ||
| * | ||
| * The visible top-level menu and its sub-items are only registered when the | ||
| * user is actually on one of the demo pages. URL access still works because | ||
| * `admin_menu` fires before WP validates the slug — `$_GET['page']` is set | ||
| * by the time `is_wizard_page()` runs, the menu registers, and WP accepts | ||
| * the page. | ||
| */ | ||
| public function add_page() { | ||
| if ( ! $this->is_wizard_page() ) { | ||
| return; | ||
| } | ||
|
|
||
| add_filter( 'parent_file', [ $this, 'set_parent_file' ] ); | ||
| add_filter( 'submenu_file', [ $this, 'set_submenu_file' ] ); | ||
|
|
||
| $icon = sprintf( | ||
| 'data:image/svg+xml;base64,%s', | ||
| base64_encode( Newspack_UI_Icons::get_svg( 'collections' ) ) | ||
| ); | ||
|
|
||
| // Anchor the menu near the top of the sidebar (just after Dashboard at pos 2) | ||
| // so the demo is easy to find — `add_menu_page` defaults to appending, which | ||
| // pushes the demo below every other plugin's menu. | ||
| add_menu_page( | ||
| $this->get_name(), | ||
| $this->get_name(), | ||
| $this->capability, | ||
| $this->slug, | ||
| [ $this, 'render_wizard' ], | ||
| $icon, | ||
| '2.5' | ||
| ); | ||
| add_submenu_page( | ||
| $this->slug, | ||
| $this->get_name(), | ||
| __( 'All Rolling Content', 'newspack-plugin' ), | ||
| $this->capability, | ||
| $this->slug, | ||
| [ $this, 'render_wizard' ] | ||
| ); | ||
| add_submenu_page( | ||
| $this->slug, | ||
| __( 'Add Rolling Content', 'newspack-plugin' ), | ||
| __( 'Add Rolling Content', 'newspack-plugin' ), | ||
| $this->capability, | ||
| self::SLUG_ADD, | ||
| [ $this, 'render_wizard' ] | ||
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Force-highlight the top-level menu when on any rolling-content page. | ||
| * Required because WP's automatic highlighting can lose the trail through | ||
| * the conditional menu registration. | ||
| * | ||
| * @param string $parent_file The current parent file. | ||
| * @return string | ||
| */ | ||
| public function set_parent_file( $parent_file ) { | ||
| return $this->slug; | ||
| } | ||
|
|
||
| /** | ||
| * Highlight the correct submenu item for the current page. | ||
| * | ||
| * @param string $submenu_file The current submenu file. | ||
| * @return string | ||
| */ | ||
| public function set_submenu_file( $submenu_file ) { | ||
| // phpcs:ignore WordPress.Security.NonceVerification.Recommended | ||
| $page = isset( $_GET['page'] ) ? sanitize_key( wp_unslash( $_GET['page'] ) ) : ''; | ||
| return in_array( $page, [ $this->slug, self::SLUG_ADD ], true ) ? $page : $submenu_file; | ||
| } | ||
|
|
||
| /** | ||
| * Enqueue scripts and styles. | ||
| * | ||
| * Replicates the relevant parts of the parent class's enqueue logic. We can't | ||
| * just call `parent::enqueue_scripts_and_styles()` because its slug check uses | ||
| * `filter_input(INPUT_GET, 'page')`, which reads from the frozen request state | ||
| * and ignores any local `$_GET` mutations — so it would bail on the SLUG_ADD | ||
| * page. | ||
| */ | ||
| public function enqueue_scripts_and_styles() { | ||
| if ( ! $this->is_wizard_page() ) { | ||
| return; | ||
| } | ||
|
|
||
| Newspack::load_common_assets(); | ||
|
|
||
| // Data carrier (no source script). | ||
| wp_register_script( 'newspack_data', '', [], '1.0', false ); | ||
|
|
||
| $plugin_data = get_plugin_data( NEWSPACK_PLUGIN_FILE ); | ||
| $urls = [ | ||
| 'dashboard' => Wizards::get_url( 'newspack-dashboard' ), | ||
| 'public_path' => Newspack::plugin_url() . '/dist/', | ||
| 'bloginfo' => [ 'name' => get_bloginfo( 'name' ) ], | ||
| 'plugin_version' => [ 'label' => $plugin_data['Name'] . ' ' . $plugin_data['Version'] ], | ||
| 'homepage' => get_edit_post_link( get_option( 'page_on_front', false ) ), | ||
| 'site' => get_site_url(), | ||
| 'support' => esc_url( 'https://help.newspack.com/' ), | ||
| 'support_email' => false, | ||
| ]; | ||
|
|
||
| $aux_data = [ | ||
| 'is_e2e' => Starter_Content::is_e2e(), | ||
| 'is_debug_mode' => Newspack::is_debug_mode(), | ||
| 'has_completed_setup' => get_option( NEWSPACK_SETUP_COMPLETE ), | ||
| 'site_title' => get_option( 'blogname' ), | ||
| 'is_managed' => method_exists( 'Newspack_Manager', 'is_connected_to_manager' ) && \Newspack_Manager::is_connected_to_manager(), | ||
| ]; | ||
|
|
||
| wp_localize_script( 'newspack_data', 'newspack_urls', $urls ); | ||
| wp_localize_script( 'newspack_data', 'newspack_aux_data', $aux_data ); | ||
| wp_enqueue_script( 'newspack_data' ); | ||
|
|
||
| wp_register_script( | ||
| 'newspack-wizards', | ||
| Newspack::plugin_url() . '/dist/wizards.js', | ||
| $this->get_script_dependencies(), | ||
| NEWSPACK_PLUGIN_VERSION, | ||
| true | ||
| ); | ||
| wp_enqueue_script( 'newspack-wizards' ); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| /** | ||
| * Rolling Content demo — shared status indicator. | ||
| * | ||
| * Renders a small icon + label for an item's status. Each caller passes its | ||
| * own labels and icons keyed by its status union. | ||
| */ | ||
|
|
||
| /** | ||
| * WordPress dependencies | ||
| */ | ||
| import { Icon } from '@wordpress/components'; | ||
|
|
||
| export default function StatusPill< S extends string >( { | ||
| status, | ||
| labels, | ||
| icons, | ||
| }: { | ||
| status: S; | ||
| labels: Record< S, string >; | ||
| icons: Record< S, JSX.Element >; | ||
| } ) { | ||
| return ( | ||
| <span style={ { display: 'inline-flex', alignItems: 'center', gap: 4 } }> | ||
| <Icon icon={ icons[ status ] } size={ 18 } /> | ||
| <span>{ labels[ status ] }</span> | ||
| </span> | ||
| ); | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not fixing this one — the parent
Wizard::enqueue_scripts_and_styles()assigns$asset_file = include …/wizards.asset.php;but never uses$asset_fileafterward;wp_register_scriptpassesNEWSPACK_PLUGIN_VERSIONand$this->get_script_dependencies()regardless. So this override is functionally equivalent to the parent. If the parent's include is meant to drive cache busting/dependencies, that would be a separate bug in the parent class.