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.

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

Fix settings form not redirecting to the correct tab when submitted #7424

Merged
merged 13 commits into from
Jan 10, 2024
14 changes: 12 additions & 2 deletions assets/css/admin-custom.scss
Original file line number Diff line number Diff line change
Expand Up @@ -276,8 +276,18 @@ div.question_boolean_fields {
}
}

.sensei-settings .sensei-custom-navigation__heading {
flex-flow: column wrap;
.sensei-settings {
.sensei-custom-navigation__heading {
flex-flow: column wrap;
}

.sensei-custom-navigation__links a {
top: 0;

&:focus {
box-shadow: none;
}
}
}

.sensei-custom-navigation__separator {
Expand Down
4 changes: 2 additions & 2 deletions assets/js/admin/course-theme/course-theme-sidebar.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ const CourseThemeSidebar = () => {
>
{ globalLearningModeEnabled ? (
<p>
<a href="/wp-admin/admin.php?page=sensei-settings#appearance-settings">
<a href="/wp-admin/admin.php?page=sensei-settings&tab=appearance-settings">
{ __(
'Learning Mode is enabled globally.',
'sensei-lms'
Expand All @@ -56,7 +56,7 @@ const CourseThemeSidebar = () => {
}
/>
<p>
<a href="/wp-admin/admin.php?page=sensei-settings#appearance-settings">
<a href="/wp-admin/admin.php?page=sensei-settings&tab=appearance-settings">
{ __( 'Change Template', 'sensei-lms' ) }
</a>
</p>
Expand Down
163 changes: 123 additions & 40 deletions assets/js/settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,134 @@ jQuery( document ).ready( function ( $ ) {
/***** Settings Tabs *****/
const $senseiSettings = $( '#woothemes-sensei.sensei-settings' );

// Show the current section.
showSection( getCurrentSectionId() );

// Switch to the section when the tab is clicked.
$senseiSettings.find( 'a.tab:not(.external)' ).on( 'click', function ( e ) {
const sectionUrl = $( this ).attr( 'href' );
const sectionId = getSectionIdFromUrl( sectionUrl );

if ( ! sectionExists( sectionId ) ) {
return true;
}

changeCurrentUrl( sectionUrl );
updateReferer( sectionUrl );
showSection( sectionId );

e.preventDefault();
} );

// Change the section when the user navigates the session history.
addEventListener( 'popstate', ( e ) => {
const sectionId = getSectionIdFromUrl( window.location.href );

if ( sectionExists( sectionId ) ) {
updateReferer( window.location.href );
showSection( sectionId );
}
} );

/**
* Change the current browser URL.
*
* @param {string} url
*/
function changeCurrentUrl( url ) {
window.history.pushState( {}, null, url );
}

/**
* Update the hidden referer field.
*
* @param {string} url
*/
function updateReferer( url ) {
const urlObject = new URL( url );

$senseiSettings.find( 'input[name="_wp_http_referer"]' )
.val( urlObject.pathname + urlObject.search );
}

/**
* Hide all sections.
*/
function hideAllSections() {
$senseiSettings.find( 'section' ).hide();
$senseiSettings.find( 'a.tab' ).removeClass( 'current' );
$senseiSettings.find( 'section' )
.hide();
}

function show( sectionId = '' ) {
$senseiSettings.find( `section#${ sectionId }` ).show();
/**
* Show a settings section.
*
* @param {string} sectionId
*/
function showSection( sectionId ) {
hideAllSections();
hideSettingsFormElements( sectionId );

$senseiSettings.find( `section#${ sectionId }` )
.show();

$senseiSettings.find( 'a.tab.current' )
.removeClass( 'current' )

$senseiSettings
.find( `[href="#${ sectionId }"]` )
.find( `a.tab[href*="tab=${ sectionId }"]` )
.addClass( 'current' );

sensei_log_event( 'settings_view', { view: sectionId } );
markSectionAsVisited( sectionId );
}

// Hide header and submit on page load if needed
hideSettingsFormElements();
/**
* Get section id from the current URL.
*
* @returns {string}
*/
function getCurrentSectionId() {
return getSectionIdFromUrl( window.location.href );
}

/**
* Get section id from a URL.
*
* @param {string} url
* @returns {string}
*/
function getSectionIdFromUrl( url ) {
const urlParams = new URLSearchParams( url );

function hideSettingsFormElements() {
const urlHashSectionId = window.location.hash?.replace( '#', '' );
if ( urlHashSectionId === 'woocommerce-settings' ) {
return urlParams.get( 'tab' )
|| url.split( '#' )[1]
|| 'default-settings';
}

/**
* Check if a section exists.
*
* @param {string} sectionId
* @returns {boolean}
*/
function sectionExists( sectionId ) {
return $( '#' + sectionId ).length > 0;
}

/**
* Hide the header and submit button if there are no settings in the section.
*
* @param {string} sectionId
*/
function hideSettingsFormElements( sectionId ) {
if ( sectionId === 'woocommerce-settings' ) {
const formRows = $senseiSettings.find( '#woocommerce-settings tr' );
// Hide header and submit if there is not settings form in section
hideHeaderAndSubmit(
! formRows.length &&
$senseiSettings.find( '#sensei-promo-banner' )
);
} else if ( urlHashSectionId === 'sensei-content-drip-settings' ) {
} else if ( sectionId === 'sensei-content-drip-settings' ) {
const formRows = $senseiSettings.find(
'#sensei-content-drip-settings tr'
);
Expand All @@ -42,6 +143,11 @@ jQuery( document ).ready( function ( $ ) {
}
}

/**
* Hide the header and submit button.
*
* @param {boolean} shouldHide
*/
function hideHeaderAndSubmit( shouldHide ) {
if ( shouldHide ) {
$senseiSettings.find( '#submit' ).hide();
Expand All @@ -52,35 +158,12 @@ jQuery( document ).ready( function ( $ ) {
}
}

window.onhashchange = hideSettingsFormElements;

// Show general settings section if no section is selected in url hasn.
const defaultSectionId = 'default-settings';
const urlHashSectionId = window.location.hash?.replace( '#', '' );
hideAllSections();
if ( urlHashSectionId ) {
show( urlHashSectionId );
} else {
show( defaultSectionId );
}

$senseiSettings.find( 'a.tab' ).on( 'click', function ( e ) {
const queryString = window.location.search;
const urlParams = new URLSearchParams( queryString );

const href = $( this ).attr( 'href' );
if ( urlParams.has( 'tab' ) || ! href?.includes( '#' ) ) {
return true;
}

e.preventDefault();
const sectionId = href.split( '#' )[ 1 ];
window.location.hash = '#' + sectionId;
hideAllSections();
show( sectionId );
return false;
} );

/**
* Mark a section as visited.
* This is used to track which sections are being used.
*
* @param {string} sectionId
*/
function markSectionAsVisited( sectionId ) {
const data = new FormData();
data.append( 'action', 'sensei_settings_section_visited' );
Expand Down
4 changes: 4 additions & 0 deletions changelog/fix-settings-form-redirect
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: patch
Type: fixed

Settings form not redirecting to the correct tab when submitted
42 changes: 2 additions & 40 deletions config/psalm/psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,6 @@
<code>$extensions</code>
<code>self::$instance</code>
</DocblockTypeContradiction>
<InvalidPropertyAssignmentValue occurrences="1">
<code>new Sensei_Setup_Wizard_Pages()</code>
</InvalidPropertyAssignmentValue>
<InvalidPropertyFetch occurrences="1">
<code>$extension-&gt;price</code>
</InvalidPropertyFetch>
Expand All @@ -324,9 +321,6 @@
<PossiblyNullPropertyFetch occurrences="1">
<code>$screen-&gt;id</code>
</PossiblyNullPropertyFetch>
<UndefinedDocblockClass occurrences="1">
<code>Sensei_Setup</code>
</UndefinedDocblockClass>
</file>
<file src="includes/admin/class-sensei-status.php">
<DocblockTypeContradiction occurrences="2">
Expand Down Expand Up @@ -463,18 +457,6 @@
<code>isset( $this-&gt;local_plugin_updates )</code>
</RedundantPropertyInitializationCheck>
</file>
<file src="includes/admin/home/quick-links/class-sensei-home-quick-links-provider.php">
<InvalidArgument occurrences="1">
<code>$this-&gt;get_email_notification_url()</code>
</InvalidArgument>
<InvalidReturnStatement occurrences="2">
<code>'/admin.php?page=sensei-settings#email-notification-settings'</code>
<code>'admin.php?page=sensei-settings&amp;tab=email-notification-settings'</code>
</InvalidReturnStatement>
<InvalidReturnType occurrences="1">
<code>array</code>
</InvalidReturnType>
</file>
<file src="includes/admin/home/tasks/class-sensei-home-tasks-provider.php">
<PossiblyInvalidPropertyFetch occurrences="1">
<code>$post-&gt;post_title</code>
Expand Down Expand Up @@ -735,18 +717,11 @@
</PossiblyFalseArgument>
</file>
<file src="includes/blocks/class-sensei-continue-course-block.php">
<DocblockTypeContradiction occurrences="1">
<code>$course_id</code>
</DocblockTypeContradiction>
<PossiblyFalseArgument occurrences="4">
<PossiblyFalseArgument occurrences="3">
<code>$course_id</code>
<code>$course_id</code>
<code>$course_id</code>
<code>get_permalink( absint( $target_post_id ?? $course_id ) )</code>
</PossiblyFalseArgument>
<RedundantConditionGivenDocblockType occurrences="1">
<code>$target_post_id</code>
</RedundantConditionGivenDocblockType>
</file>
<file src="includes/blocks/class-sensei-course-blocks.php">
<PossiblyInvalidArgument occurrences="1">
Expand Down Expand Up @@ -1080,10 +1055,6 @@
<TooManyArguments occurrences="1">
<code>esc_html( sprintf( __( 'Please supply a %1$s ID.', 'sensei-lms' ) ), $post_type )</code>
</TooManyArguments>
<UndefinedDocblockClass occurrences="2">
<code>Sensei()-&gt;setup_wizard-&gt;pages</code>
<code>Sensei()-&gt;setup_wizard-&gt;pages</code>
</UndefinedDocblockClass>
</file>
<file src="includes/class-sensei-analysis-course-list-table.php">
<ArgumentTypeCoercion occurrences="1"/>
Expand Down Expand Up @@ -2877,9 +2848,7 @@
<DeprecatedProperty occurrences="1">
<code>$this-&gt;file</code>
</DeprecatedProperty>
<DocblockTypeContradiction occurrences="7">
<code>! $encoded_feedback</code>
<code>! $encoded_feedback</code>
<DocblockTypeContradiction occurrences="5">
<code>! is_array( $quiz_answers )</code>
<code>! is_array( $quiz_answers )</code>
<code>! is_array( $unprepared_answers )</code>
Expand Down Expand Up @@ -3896,7 +3865,6 @@
<code>! self::$instance</code>
<code>self::$instance</code>
</DocblockTypeContradiction>
<InvalidArgument occurrences="1"/>
<MissingPropertyType occurrences="1">
<code>Sensei()-&gt;plugin_url</code>
</MissingPropertyType>
Expand All @@ -3915,9 +3883,6 @@
<PropertyNotSetInConstructor occurrences="1">
<code>$original_theme</code>
</PropertyNotSetInConstructor>
<UndefinedDocblockClass occurrences="1">
<code>The</code>
</UndefinedDocblockClass>
</file>
<file src="includes/course-video/blocks/class-sensei-course-video-blocks-embed-extension.php">
<DeprecatedMethod occurrences="2">
Expand Down Expand Up @@ -5983,9 +5948,6 @@
<PropertyNotSetInConstructor occurrences="1">
<code>Sensei_REST_API_Setup_Wizard_Controller</code>
</PropertyNotSetInConstructor>
<UndefinedDocblockClass occurrences="1">
<code>$this-&gt;setup_wizard-&gt;pages</code>
</UndefinedDocblockClass>
</file>
<file src="includes/rest-api/class-sensei-rest-api-theme-controller.php">
<ArgumentTypeCoercion occurrences="1"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@
__( 'Settings', 'sensei-lms' ),
[
$this->create_item( __( 'Email notifications', 'sensei-lms' ), admin_url( $this->get_email_notification_url() ) ),
$this->create_item( __( 'Learning mode', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings#appearance-settings' ) ),
$this->create_item( __( 'WooCommerce', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings#woocommerce-settings' ) ),
$this->create_item( __( 'Content drip', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings#sensei-content-drip-settings' ) ),
$this->create_item( __( 'Learning mode', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings&tab=appearance-settings' ) ),
$this->create_item( __( 'WooCommerce', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings&tab=woocommerce-settings' ) ),
$this->create_item( __( 'Content drip', 'sensei-lms' ), admin_url( '/admin.php?page=sensei-settings&tab=sensei-content-drip-settings' ) ),
]
),
$this->create_category(
Expand All @@ -53,14 +53,14 @@
/**
* Return the correct email notification settings based on the feature flag.
*
* @return array The magical link to create a demo course or the link to edit the demo course.
* @return string The magical link to create a demo course or the link to edit the demo course.
*/
private function get_email_notification_url() {
if ( Sensei()->feature_flags->is_enabled( 'email_customization' ) ) {
return 'admin.php?page=sensei-settings&tab=email-notification-settings';
}

return '/admin.php?page=sensei-settings#email-notification-settings';
return '/admin.php?page=sensei-settings&tab=email-notification-settings';

Check warning on line 63 in includes/admin/home/quick-links/class-sensei-home-quick-links-provider.php

View check run for this annotation

Codecov / codecov/patch

includes/admin/home/quick-links/class-sensei-home-quick-links-provider.php#L63

Added line #L63 was not covered by tests
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
* @return string
*/
public function get_url(): ?string {
return admin_url( 'admin.php?page=sensei-settings#appearance-settings' );
return admin_url( 'admin.php?page=sensei-settings&tab=appearance-settings' );

Check warning on line 49 in includes/admin/home/tasks/task/class-sensei-home-task-configure-learning-mode.php

View check run for this annotation

Codecov / codecov/patch

includes/admin/home/tasks/task/class-sensei-home-task-configure-learning-mode.php#L49

Added line #L49 was not covered by tests
}

/**
Expand Down
Loading
Loading