[#40994] - [5.1] Guided Tours - Add checkbox/radio/select target element support, enhanced required field handling
- Fixed in Code Base
- 19 Feb 2024
- Medium
- Build: 5.0-dev
- # 40994
- Diff
- GeraintEdwards:tour-4
User tests: Successful: Unsuccessful:
Pull Request for Issue # .
Summary of Changes
- Add checkbox/radio/select target element support for interactive field types
- Add support for textarea target element alongside input text type targets
- Allow fields that are not 'required' in the DOM to be required in the field e.g. for tours relating to searching where the input field is required for the tour but not by Joomla or the active page component
- Allow the tour to insist on a specific required value (translatable) so that the context of the tour can be maintained as it progresses e.g. searching for a specific text value.
Please note that a new database field has been added in this PR 'params' in the guided tours steps table.
Testing Instructions
Test the provided tour.
Check that old tours still function properly.
Actual result BEFORE applying this Pull Request
The tour will not respond to the checkbox, select list and textarea field changes correctly
Expected result AFTER applying this Pull Request
See the tour as it plays out in these screenshots
Title: Global config demo tour
URL: /administrator/index.php?option=com_config
Description:
<p>This tour will demonstrate:</p>
<ul>
<li>checkboxes</li>
<li>radio boxes</li>
<li>required text fields searching for specific value</li>
</ul>
Step 1
Title: Change editor to CodeMirror
Description: You should change the default editor and select 'CodeMirror'
Target: #jform_editor
Type: interactive
Interactive Type: select list
Options: value required and value to enter: codemirror.
Note: all steps should have the position 'bottom'.
Step 2
Title: Put the site offline
Description: Now put the site offline
Target: #jform_offline1
Type: interactive
Interactive Type: checkbox/radio
Options: value required.
Step 3
Title: Change custom message
Description: Please set the custom message to: We will be back soon.
Target: #jform_offline_message
Type: interactive
Interactive Type: text field
Options: value required and value to enter: We will be back soon.
Step 4
Title: Check the frontend of the site
Description: The tour is now finished - save the configuration and check the site in the frontend
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
-
No documentation changes for docs.joomla.org needed
-
Pull Request link for manual.joomla.org:
-
No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | Administration Language & Strings JavaScript Repository NPM Change SQL Installation Front End Plugins |
| Status | New | ⇒ | Pending |
| Labels |
Added:
Language Change
NPM Resource Changed
PR-5.0-dev
|
||
| Title |
|
||||||
- The new "params" column has to be added to the extensions.sql for PostgreSQL, too.
- It needs one update SQL script for each kind of database (MySQl and PotsgreSQL) to add the new column to the table.
- Fix PHP code style errors: We use 4 spaces for each level of indentation in PHP files, not tabs.
- Fix XML code style.
Thanks @richard67 - I've fixed the missing postgres column, PHP code style and I think I fixed the XML file (the code style guidlines are a bit vague at https://developer.joomla.org/coding-standards/xml.html)
I'm not sure how I would add an update script to add the existing column - I could find no examples in the 5.0dev codebase. Can you point me in the right direction of suggest an example to look at?
I've fixed the missing postgres column
I can't see that yet.Have you forgotten to push the change?
I'm not sure how I would add an update script to add the existing column - I could find no examples in the 5.0dev codebase. Can you point me in the right direction of suggest an example to look at?
In the 5.0-dev coce base you won't find much because we removed all the 4.x update SQL scripts. But you can find some examples in the 4.4-dev or 4.3-dev code base:
| Category | Administration Language & Strings JavaScript Repository NPM Change SQL Installation Front End Plugins | ⇒ | Administration Language & Strings JavaScript Repository NPM Change SQL Installation Postgresql Front End Plugins |
P.S.: The new update SQL script should have a version 5.0.0 and a timestamp newer that the present script "5.0.0-2023-03-11.sql", so I suggest to use "5.0.0-2023-06-21.sql".
| Category | Administration Language & Strings JavaScript Repository NPM Change SQL Installation Front End Plugins Postgresql | ⇒ | SQL Administration com_admin Postgresql Language & Strings JavaScript Repository NPM Change Installation Front End Plugins |
Whoops - postgres change was not pushed :(
Thanks for the update SQL location.
For some reason my PHPStorm installation hadn't picked up on these tabs when I analysed the code :(
Whoops - postgres change was not pushed :(
Thanks for the update SQL location.
For some reason my PHPStorm installation hadn't picked up on these tabs when I analysed the code :(
There are more code style problems with tabs instead of spaces, but I'm busy at work so can't check further.
If you click the "Details" link at the right side of the "continuous-integration/drone/p" step at the bottom of the PR you come to a page with the CI checks. There you select "PHPCS" at the left hand side, then you see the log of the PHP code style check.
This pull request has been automatically rebased to 5.1-dev.
| Labels |
Added:
Feature
|
||
| Title |
|
||||||
| Labels |
Added:
PR-5.1-dev
|
||
I have tested this item ✅ successfully on 20c8ac4
Worked as expected for the toggle, dropdown and required text.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40994.
| Labels |
Added:
Updates Requested
Removed: PR-5.0-dev |
||
This PR is being discussed during our Guided Tours meetings and suggested improvements are included:
- the wording of the parameters/naming of the fields
- adding a message (note field) to the Options tab to force the tab to exist always (when the options tab is open and a new step is created, the user is greeted with a blank section because the tab is missing).
An issue was found when a radio button's value is NO and is required to be set to YES to go further:
when the radio button is set to YES (the 'next' button becomes available) then set to NO again, the 'next' button is not going back to being disabled, the user can go through to the next step even though the radio button is set to NO.
Is there any way you could look at it @GeraintEdwards ?
That said, it would be nice to force a radio button to have a NO value before being able to get to the next step (we should allow the required value field to be visible for radio/checkbox and specify 0 or 1 as the required value). But that could be an addition to this functionality later on.
I have tested this item ✅ successfully on 20c8ac4
Ok for interactives Steps (RadioButton, text, list, button) and Ok for the Options Tab with info text.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40994.
I have tested this item ✅ successfully on 20c8ac4
Retested successfully after changes
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40994.
I have tested this item ✅ successfully on 20c8ac4
Test was successfull.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40994.
@obuisard Please rename the update SQL scripts like I suggested in my review comment.
Thanks for the reminder Richard @richard67, I totally missed that one :-)
Thanks for the reminder Richard @richard67, I totally missed that one :-)
@obuisard Maybe my mistake because I just saw I had not finished that review with that comment so the review was still ongoing and maybe only me could see it.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40994.
| Labels |
Added:
RTC
|
||
Thanks for the reminder Richard @richard67, I totally missed that one :-)
@obuisard Maybe my mistake because I just saw I had not finished that review with that comment so the review was still ongoing and maybe only me could see it.
No, no, my fault, I should know better :-)
| Labels |
Removed:
Updates Requested
|
||
@GeraintEdwards @obuisard We should not use a NOT NULL constraint for the new column, even if other tables have that for their params column. But e.g. the #__categories table allows NULL values for its params column.
I have made corresponding review comments with change suggestions for that.
The reason is following:
When we add the new column to the table on updates with a NOT NULL constraint, it works on MySQL and MariaDB because that implicitly sets the value to an empty string for that new column.
But on PostgreSQL that does not work. We have to add the column first without the NOT NULL constraint, then update the existing records, and then add the NOT NULL CONSTRAINT. If this fails for some reason, the database fixer will not be able to fix that because it will only run the 2 ALTER TABLE statements but not the UPDATE statement between them, so the 2nd ALTER TABLE will always fail.
Therefore it's the best to allow NULLs, and as far as I can see in the PHP code of this PR it handles that already. But this has to be tested in order to be safe.
| Labels |
Added:
Updates Requested
|
||
@GeraintEdwards @obuisard We should not use a NOT NULL constraint for the new column, even if other tables have that for their
paramscolumn. But e.g. the#__categoriestable allows NULL values for itsparamscolumn.I have made corresponding review comments with change suggestions for that.
The reason is following:
When we add the new column to the table on updates with a NOT NULL constraint, it works on MySQL and MariaDB because that implicitly sets the value to an empty string for that new column.
But on PostgreSQL that does not work. We have to add the column first without the NOT NULL constraint, then update the existing records, and then add the NOT NULL CONSTRAINT. If this fails for some reason, the database fixer will not be able to fix that because it will only run the 2 ALTER TABLE statements but not the UPDATE statement between them, so the 2nd ALTER TABLE will always fail.
Therefore it's the best to allow NULLs, and as far as I can see in the PHP code of this PR it handles that already. But this has to be tested in order to be safe.
Thank you so much Richard @richard67 for the explanation, it does make perfect sense. Really appreciate your insight on this!
| Labels |
Removed:
Updates Requested
|
||
Expand the description to include Checkbox/Radio and Select List?
Expand the description to include
Checkbox/RadioandSelect List?
Good point, thanks!
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2024-02-19 16:41:36 |
| Closed_By | ⇒ | LadySolveig |
Thank you so much for this very useful improvements! ? @GeraintEdwards @obuisard and for the nice teamwork and support to @Quy @richard67
Good afternoon. I’m not sure yet, I’m still figuring it out, but...
When creating a step, I select the interactive and checkbox/radio, but I don't see the target value in the Target Value Options tab (similar to other interactives).
Is this expected behavior or a bug?
As far as I understand, from the selection list I can NOT select everything, but only some items.
Good afternoon. I’m not sure yet, I’m still figuring it out, but...
When creating a step, I select the interactive and checkbox/radio, but I don't see the target value in the Target Value Options tab (similar to other interactives).
Is this expected behavior or a bug?
As far as I understand, from the selection list I can NOT select everything, but only some items.
At present the code allows one selector - the specific checkbox/radiobox and the required option needs this to be checked. I guess it may make sense to generalise this so allow for the requirement that the checkbox/radiobox to be unchecked and also to allow require multiple options to be checked (or selected in select list fields). This is more complex to implement as the values are not available to us when editing the steps.
It would, of course, be really neat if we could create/edit the tour and its steps in-situ so that we could pick up the values from the form in real time using javascript. But I think that is a step to advanced for now.