Fix settings form not redirecting to the correct tab when submitted

[m1r0]

Resolves #7351
Related to #7415

Proposed Changes

This PR fixes the settings form always redirecting to the first (General) tab when the form is saved.

It also fixes the active tab highlight not working on the tabs menu.

It changes the URL structure to using params instead of hash (#default-settings -> &tab=default-settings) but the old structure is still supported.

Testing Instructions

1. Go to Sensei LMS -> Settings -> Appearance (you could choose any tab that uses a hash in the URL except the first one)
2. Click the "Save Changes" button.
3. You should be redirected to the same tab and not to the first one (General).
4. Check if the active tab is highlighted in the tabs menu.
5. Check if the Emails tab is working as before.
6. Check if the old hash URL structure is still working.

Pre-Merge Checklist

• PR title and description contain sufficient detail and accurately describe the changes
• Acceptance criteria is met
• Decisions are publicly documented
• Adheres to coding standards (PHP, JavaScript, CSS, HTML)
• All strings are translatable (without concatenation, handles plurals)
• Follows our naming conventions (P6rkRX-4oA-p2)
• Hooks (p6rkRX-1uS-p2) and functions are documented
• New UIs are responsive and use a mobile-first approach
• New UIs match the designs
• Different user privileges (admin, teacher, subscriber) are tested as appropriate
• Legacy courses (course without blocks) are tested
• Code is tested on the minimum supported PHP and WordPress versions
• User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
• "Needs Documentation" label is added if this change requires updates to documentation
• Known issues are created as new GitHub issues

[github-actions]

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit c801397 and trunk. Please review and confirm the following are correct before merging.

Script Handle	Added Dependencies	Removed Dependencies	Total Size	Size Diff
js/admin/course-edit.js			10.2 kB	+5 B ( +0.05% 🔼 )
js/settings.js			1.17 kB	+269 B ( +29.93% 🔼 )

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (06384bd) 51.04% compared to head (c801397) 51.14%.
Report is 3 commits behind head on trunk.

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk    #7424      +/-   ##
============================================
+ Coverage     51.04%   51.14%   +0.10%     
- Complexity    11167    11169       +2     
============================================
  Files           614      614              
  Lines         47148    47152       +4     
  Branches        405      405              
============================================
+ Hits          24066    24117      +51     
+ Misses        22755    22708      -47     
  Partials        327      327              

Files	Coverage Δ
includes/class-sensei-lesson.php	36.20% <100.00%> (ø)
includes/class-sensei-settings-api.php	25.95% <100.00%> (+9.96%)	⬆️
includes/internal/emails/class-settings-menu.php	100.00% <100.00%> (ø)
...k-links/class-sensei-home-quick-links-provider.php	95.91% <75.00%> (ø)
...class-sensei-home-task-configure-learning-mode.php	27.27% <0.00%> (ø)
includes/class-sensei-course.php	38.95% <0.00%> (ø)

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eff6284...c801397. Read the comment docs.

[renatho]

Hi @m1r0! I didn't test it, but I suspect we can also close #7351 with this PR.

[m1r0]

Thanks! I've updated the PR description.

[Imran92]

Hi @m1r0 👋

This looks good to me overall and is working as expected. I just wanted to discuss another approach that won't involve session storage and can potentially help with testing.

What if we add a hidden input field here in this form

sensei/includes/class-sensei-settings-api.php

Line 540 in 51c2119

<form id="<?php echo esc_attr( $this->token ); ?>-form" action="options.php" method="post">

that we update the value of when a settings tab is selected. Then when it's submitted, we can set the current tab from the backend here

sensei/includes/class-sensei-settings-api.php

Line 306 in 51c2119

if ( 'default-settings' === $k ) {

, maybe with a code like this -

$html .= '<ul id="settings-sections" class="subsubsub hide-if-no-js">' . "\n";
			$current_tab = isset( $_POST['tab'] ) ? sanitize_text_field( wp_unslash( $_POST['tab'] ) ) : 'default-settings';

			$sections = array();

			foreach ( $this->tabs as $k => $v ) {
				$classes = 'tab';
				if ( $current_tab === $k ) {
					$classes .= ' current';
				}

Also, currently, the Current tab highlight isn't working because of a bug here -

sensei/assets/js/settings.js

Lines 12 to 14 in 51c2119

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

We just need to replace href= with href$=. And also should update the logic here a bit so that it doesn't remove the 'current' class in every case

sensei/assets/js/settings.js

Line 7 in 51c2119

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

LMKWDYT!

[m1r0]

Thanks for the nice suggestion, Imran. ❤️

I did try your idea but unfortunately, I've stumbled on the issue that the POST data is not accessible when the settings tabs are rendered. The form is submitting the data to options.php and there is a redirect back to the settings URL, so the POST data is lost. Have you tested the suggested code? Am I missing something?

Maybe it would be best to just not use JS to switch the tabs and move that responsibility to the backend. That will clean up all the messy tabs/hash logic but it will require some time to do.

[Imran92]

Maybe it would be best to just not use JS to switch the tabs and move that responsibility to the backend

Absolutely agreed. That'd be best. But we'd still have to depend on JS a bit for the tabs. Otherwise we'd have to reload each time, which won't be a pleasant experience. But we should try to keep it at minimum.

The form is submitting the data to options.php and there is a redirect back to the settings URL, so the POST data is lost

Oh, in that case, we can just change the value of the input field _wp_http_referer and that will start working. We can either add the hash of the tab in the referer, or we can use a query param. Added advantage of this approach is we don't need local storage, using referer URL to guide to the intended page is more conventional and expected. Also, we can set the 'current' tab from the backend in that approach. I've pushed a quick POC branch that's working with this approach. LMK what you think about this approach

[m1r0]

@Imran92, great input as always!

I've reworked the settings tabs to use &tab=default-settings URL structure, taking your suggestions into account. It makes everything a lot simpler.

The navigation between the tabs is not refreshing the page, so it works as before. The old hash URL structure is also working so it's backwards compatible.

As a bonus, you can now navigate the tabs using the browser history, so navigating back/forward should work. 🙂

LMKWYT.

[Imran92]

Thanks for making the changes @m1r0 ! Looks good and works as expected <3