Automattic / sensei

Sensei LMS - Online Courses, Quizzes, & Learning
https://senseilms.com
GNU General Public License v2.0
545 stars 198 forks source link

Undocumented changes to My Courses tabs #1979

Open braders opened 6 years ago

braders commented 6 years ago

This issue affects the Sensei "My Courses" page. (That is, the page configured to use the [sensei_user_courses] shortcode).

What I Expected

It appears that multiple changes were made in the latter half of last year.

  1. An additional "All Courses" tab was added - There should be some way to turn of this new functionality to revert to the previous behaviour of two tabs (Active and Completed).
  2. The front-end filtering used to work with JavaScript, without requiring a full page load. This no longer works. It is unclear if this is intentional, however the old (inline) logic for this is still written out to the page. It appears that this front-end filtering has stopped working due to the IDs of these tab elements changing.
  3. In our particular case we are hooking into WordPress's gettext filters, in order to change the text of the Active and Completed tabs, including writing out spans to better format the text. Since the last release this no longer works due to the tab text being run through esc_html.

Our ideal is that it would be possible to filter the avaliable tabs (including the html text, classes and Ids).

WordPress / Sensei version

Sensei 1.9.19 (Updating from 1.9.14)

Browser / OS version

Firefox Developer Edition

Context

Our client has grown used to the old behaivour, and if we cannot reproduce this on Sensei 1.9.19, then we will be forced to keep using the old version of this plugin. Any help you can provide would be greatly appreciated.

donnapep commented 6 years ago

An additional "All Courses" tab was added - There should be some way to turn of this new functionality to revert to the previous behaviour of two tabs (Active and Completed).

As far as I'm aware, the All Courses tab has always been visible except in the case where the status attribute is set to either active or complete. If that's not the case, could you share what your shortcode looks like?

The front-end filtering used to work with JavaScript, without requiring a full page load. This no longer works.

This will be fixed in Sensei 1.9.20.

In our particular case we are hooking into WordPress's gettext filters, in order to change the text of the Active and Completed tabs, including writing out spans to better format the text. Since the last release this no longer works due to the tab text being run through esc_html.

We could perhaps consider adding filters for these, but I'd like to understand the need a bit better. In particular, why do you feel it's necessary to use spans to format the text? Is the current design lacking in some way?

Thx.

braders commented 6 years ago

An additional "All Courses" tab was added - There should be some way to turn of this new functionality to revert to the previous behaviour of two tabs (Active and Completed).

As far as I'm aware, the All Courses tab has always been visible except in the case where the status attribute is set to either active or complete. If that's not the case, could you share what your shortcode looks like?

From what I can see this was added in pull request #1919. Previously two links were used, whereas now there are 3 items in a new $active_filter_options array.

The front-end filtering used to work with JavaScript, without requiring a full page load. This no longer works.

This will be fixed in Sensei 1.9.20.

Great. Any idea when this might be released?

In our particular case we are hooking into WordPress's gettext filters, in order to change the text of the Active and Completed tabs, including writing out spans to better format the text. Since the last release this no longer works due to the tab text being run through esc_html.

We could perhaps consider adding filters for these, but I'd like to understand the need a bit better. In particular, why do you feel it's necessary to use spans to format the text? Is the current design lacking in some way?

In our particular case we are using spans to reduce the amount of text shown on mobile (where the tabs would otherwise not fit in a single line). First image is desktop, second image mobile.

2018-01-30_09h54_11 2018-01-30_09h54_32

I appreciate this is a bit of an edge case, and that there are other ways we could work around the current implementation - It feels like this is a reasonable thing to want to do though, so it would be great to have an easy way to acheive this.

Thanks.