Fix settings form not redirecting to the correct tab when submitted

assets/css/admin-custom.scss

@@ -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 {

assets/js/settings.js

@@ -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'
 			);
@@ -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();
@@ -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' );

changelog/fix-settings-form-redirect

@@ -0,0 +1,4 @@
+Significance: patch
+Type: fixed
+
+Settings form not redirecting to the correct tab when submitted

includes/class-sensei-settings-api.php

@@ -299,18 +299,24 @@ public function settings_tabs() {
 			$html .= '<ul id="settings-sections" class="subsubsub hide-if-no-js">' . "\n";
 
 			$sections = array();
+			// phpcs:ignore WordPress.Security.NonceVerification.Recommended -- Nonce verification is not required here.
+			$current_tab = isset( $_GET['tab'] ) ? sanitize_key( wp_unslash( $_GET['tab'] ) ) : 'default-settings';
 
 			foreach ( $this->tabs as $k => $v ) {
 				$classes = 'tab';
 
-				if ( 'default-settings' === $k ) {
+				if ( $current_tab === $k ) {
 					$classes .= ' current';
 				}
 
+				if ( ! empty( $v['external'] ) ) {
+					$classes .= ' external';
+				}
+
 				$sections[ $k ] = array(
 					'href'  => isset( $v['href'] )
 						? esc_attr( $v['href'] )
-						: admin_url( 'admin.php?page=' . $this->token . '#' . esc_attr( $k ) ),
+						: admin_url( 'admin.php?page=' . $this->token . '&tab=' . esc_attr( $k ) ),
 					'name'  => esc_attr( $v['name'] ),
 					'class' => esc_attr( $classes ),
 				);

includes/internal/emails/class-settings-menu.php

@@ -44,6 +44,7 @@ public function replace_email_tab( array $sections ) {
 			'name'        => __( 'Emails', 'sensei-lms' ),
 			'description' => __( 'Settings for email notifications sent from your site.', 'sensei-lms' ),
 			'href'        => admin_url( 'admin.php?page=sensei-settings&tab=email-notification-settings' ),
+			'external'    => true,
 		);
 		return $sections;
 	}

tests/unit-tests/internal/emails/test-class-settings-menu.php

@@ -35,6 +35,7 @@ public function testReplaceEmailTab_SectionsGiven_ReplacesEmailNoficationSetting
 				'name'        => 'Emails',
 				'description' => 'Settings for email notifications sent from your site.',
 				'href'        => admin_url( 'admin.php?page=sensei-settings&tab=email-notification-settings' ),
+				'external'    => true,
 			],
 		];
 		self::assertSame( $expected, $sections );

tests/unit-tests/test-class-sensei-settings-api.php

@@ -1,6 +1,8 @@
 <?php
 namespace SenseiTest;
 
+use Sensei_Settings_API;
+
 /**
  * File with class for testing Sensei Settings API.
  *
@@ -24,7 +26,7 @@ protected function tearDown(): void {
 
 	public function testFormFieldText_MultipleSetToTrue_OutputsMupltipleProperty() {
 		/** Arrange. */
-		$settings = new \Sensei_Settings_Api();
+		$settings = new Sensei_Settings_Api();
 
 		/** Act. */
 		ob_start();
@@ -44,7 +46,7 @@ public function testFormFieldText_MultipleSetToTrue_OutputsMupltipleProperty() {
 
 	public function testFormFieldText_MultipleNotSet_DoesntOutputMupltipleProperty() {
 		/** Arrange. */
-		$settings = new \Sensei_Settings_Api();
+		$settings = new Sensei_Settings_Api();
 
 		/** Act. */
 		ob_start();
@@ -58,4 +60,73 @@ public function testFormFieldText_MultipleNotSet_DoesntOutputMupltipleProperty()
 		/** Assert. */
 		$this->assertStringNotContainsString( ' multiple ', $output );
 	}
+
+	public function testSettingsTabs_WhenNoTabParam_AddsTheCurrentClassToTheDefaultTabLink() {
+		/** Arrange. */
+		$settings           = new Sensei_Settings_Api();
+		$settings->has_tabs = true;
+		$settings->sections = array(
+			'default-settings' => array(
+				'name' => 'Default Settings',
+			),
+			'other-settings'   => array(
+				'name' => 'Other Settings',
+			),
+		);
+
+		/** Act. */
+		ob_start();
+		$settings->general_init();
+		$settings->settings_tabs();
+		$tabs = ob_get_clean();
+
+		/** Assert. */
+		$this->assertStringContainsString( '<a href="http://example.org/wp-admin/admin.php?page=sensei-settings&#038;tab=default-settings" class="tab current">Default Settings</a>', $tabs );
+	}
+
+	public function testSettingsTabs_WhenHasTabParam_AddsTheCurrentClassToTheTabLink() {
+		/** Arrange. */
+		$settings           = new Sensei_Settings_Api();
+		$settings->has_tabs = true;
+		$settings->sections = array(
+			'default-settings' => array(
+				'name' => 'Default Settings',
+			),
+			'other-settings'   => array(
+				'name' => 'Other Settings',
+			),
+		);
+
+		$_GET['tab'] = 'other-settings';
+
+		/** Act. */
+		ob_start();
+		$settings->general_init();
+		$settings->settings_tabs();
+		$tabs = ob_get_clean();
+
+		/** Assert. */
+		$this->assertStringContainsString( '<a href="http://example.org/wp-admin/admin.php?page=sensei-settings&#038;tab=other-settings" class="tab current">Other Settings</a>', $tabs );
+	}
+
+	public function testSettingsTabs_WhenTabIsExternal_AddsTheExternalClassToTheTabLink() {
+		/** Arrange. */
+		$settings           = new Sensei_Settings_Api();
+		$settings->has_tabs = true;
+		$settings->sections = array(
+			'other-settings' => array(
+				'name'     => 'Other Settings',
+				'external' => true,
+			),
+		);
+
+		/** Act. */
+		ob_start();
+		$settings->general_init();
+		$settings->settings_tabs();
+		$tabs = ob_get_clean();
+
+		/** Assert. */
+		$this->assertStringContainsString( '<a href="http://example.org/wp-admin/admin.php?page=sensei-settings&#038;tab=other-settings" class="tab external">Other Settings</a>', $tabs );
+	}
 }