Skip to content
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.

Sign up for GitHub

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

RDV: Use the tour only on a subset of the pages #40687

Closed
wants to merge 3 commits into from
Closed

RDV: Use the tour only on a subset of the pages #40687

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cd94ed2
RDV: Use the tour only on a subset of the pages
BogdanUngureanu Dec 19, 2024
01d26a3
Fix typo
BogdanUngureanu Dec 19, 2024
61fe737
Duplicate the list
BogdanUngureanu Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions projects/packages/jetpack-mu-wpcom/changelog/fix-tour-on-subset-of-rdv
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Enable the tour only on a subset of the Removed duplicate views screens.
11 changes: 7 additions & 4 deletions ...ts/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,16 +116,19 @@
}
add_filter( 'pre_update_option_wpcom_admin_interface', 'wpcom_admin_interface_pre_update_option', 10, 2 );

const WPCOM_DUPLICATED_VIEW = array(
const WPCOM_DUPLICATE_VIEW_WITH_TOUR = array(
Copy link
Member

@mmtr mmtr Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think the tour is the differentiating factor here. The "View" switcher is also hidden on all these screens, and it should remain visible on screens like admin.php?page=stats or tools.php?page=advertising.

Actually, I think adding admin.php?page=stats and tools.php?page=advertising to they array of duplicate views was a mistake, because we are not removing the duplicate Calypso screens for those (which is why we don't show the tour and why we don't hide the switcher).

Due to that, I think it'd be less confusing if we have these two arrays:

  • WPCOM_DUPLICATE_VIEW: Lists of WP Admin paths whose Calypso counterpart has been removed, and therefore we need to:
    • Enforce the WP Admin link in the menu
    • Hide the View switcher
    • Show the tour
  • WPCOM_CLASSIC_INTERFACE_PATHS: List of WP Admin paths not enabled for the Default admin interface (e.g. Jetpack Stats, Advertising Tools) that can be accessed from a enforced WP Admin view
    • Access to these paths should be enabled when visiting a WP Admin view whose Calypso counterpart has been removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mmtr this PR it's now redundant and replaced by #40690.

I should've closed it yesterday, but forgot. Sorry!

'edit.php',
'admin.php?page=stats',
'tools.php?page=advertising',
'edit.php?post_type=jetpack-portfolio',
'edit.php?post_type=jetpack-testimonial',
'edit-comments.php',
'edit-tags.php?taxonomy=category',
'edit-tags.php?taxonomy=post_tag',
);
const WPCOM_DUPLICATED_VIEW = array(

Check warning on line 127 in projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php

View workflow job for this annotation

GitHub Actions / PHP Code Sniffer (non-excluded files only) 

Equals sign not aligned with surrounding assignments; expected 10 spaces but found 1 space (Generic.Formatting.MultipleStatementAlignment.NotSameWarning)
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like we're going to need to duplicate the previous array

...WPCOM_DUPLICATE_VIEW_WITH_TOUR,

Check failure on line 128 in projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php

View workflow job for this annotation

GitHub Actions / dev branch for PHP 8.0 

Array unpacking within array declarations using the spread operator is not supported in PHP 7.3 or earlier. Found: ...WPCOM_DUPLICATE_VIEW_WITH_TOUR (PHPCompatibility.Syntax.NewArrayUnpacking.Found)

Check failure on line 128 in projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php

View workflow job for this annotation

GitHub Actions / PHP Compatibility 

Array unpacking within array declarations using the spread operator is not supported in PHP 7.3 or earlier. Found: ...WPCOM_DUPLICATE_VIEW_WITH_TOUR (PHPCompatibility.Syntax.NewArrayUnpacking.Found)

Check failure on line 128 in projects/packages/jetpack-mu-wpcom/src/features/wpcom-admin-interface/wpcom-admin-interface.php

View workflow job for this annotation

GitHub Actions / PHP lint (7.2) 

Parse error: syntax error, unexpected '...' (T_ELLIPSIS), expecting ')'
'admin.php?page=stats',
'tools.php?page=advertising',
);

/**
* Get the current screen section.
Expand Down Expand Up @@ -462,7 +465,7 @@

$current_screen = wpcom_admin_get_current_screen();

if ( ! in_array( $current_screen, WPCOM_DUPLICATED_VIEW, true ) ) {
if ( ! in_array( $current_screen, WPCOM_DUPLICATE_VIEW_WITH_TOUR, true ) ) {
return;
}

Expand Down
Loading