[#41303] - [5.0] [Guided Tours] Add context to core tours
- Fixed in Code Base
- 2 Sep 2023
- Medium
- Build: 5.0-dev
- # 41303
- Diff
- obuisard:guided-tours-context
User tests: Successful: Unsuccessful:
Pull Request for Issues #40023 and #41273.
Summary of Changes
Addition of context to the core tours.
Testing Instructions
Check the list of tours on different pages of the administration.
When going to 'Articles', the list of tours should present the article and the categories tours in priority.
Special case with categories:
Articles and Categories tours are now showing in each other's context.
Create a duplicate of the categories tour.
Change the title to 'Contact category tour'.
Change the url to administrator/index.php?option=com_categories&view=categories&extension=com_contact.
In component selector, select 'categories' and 'contacts'.
Publish the tour and save.
Go to the contacts tour.
Add the 'categories' to the component selector. Save.
You should now have the new category tour available to the contacts and contact categories as tours that show in priority in the list of tours.
Check the articles and article categories page to make sure the new category tour for contacts is NOT present.
Check the feeds category page to make sure no tour has priority there.
Actual result BEFORE applying this Pull Request
The numbers of tours showing in the dropdown can go up to X items, X being set in the tours administrator module.
Only the guided tours have context and show first in the list of tours when the user is listing tours or steps.
The tours follow the order in which they have been ordered in the Guided Tours component.
Expected result AFTER applying this Pull Request
On any given page where the module 'Take a tour' is showing, the tours being in the context of the page are the ones showing in priority.
The tours are still following the order that is set in the Guided Tours component. The only difference is that now there is a pre-list of tours, more easily in reach when a tour exists for the context the user is in.
A tour having the parameter 'extensions' set to 'all' will always show in the top list.
Then are shown the next X tours, X being specified in the administrator module for the tours. Here X is set to 5. The contextual tours are not part of the count.
If X is set to 0, only the contextual tours will show, which gives administrators the possibility to only show contextual tours in the list of tours.
Link to documentations
Please select:
-
Documentation link for docs.joomla.org:
Admin Module: Guided Tours
Guided Tours: New or Edit Tour -
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 | ⇒ | SQL Administration com_admin Postgresql Modules Installation |
| Status | New | ⇒ | Pending |
| Labels |
Added:
PR-5.0-dev
|
||
You realise that with this change the articles category tour is now emphasised in every component that supports categories?
The markup for this is invalid as you have the
<hr>seperator as a listitem so it is included in the count of list items but there is nothing to anounce.What you are actually creating is multiple lists and should be marked up a such.
This stackoverflow desccribes it perfectly
This is how the markup is documented at Bootstrap (https://getbootstrap.com/docs/5.2/components/dropdowns/#dividers)... So I guess I should take it with a grain of salt and I could separate the lists... Or I'll do like with the sidebar menu and have the li with class 'divider' and role 'presentation'.
You realise that with this change the articles category tour is now emphasised in every component that supports categories?
It does. it's a problem with categories. The category tour applies to articles in particular but it's the same tour for all components. If I only give the category tour a context 'com_categories', it still shows under 'Banner' categories. The only way this does not happen is to have specific code in the module that handles categories differently. I will just make the module 'smarter' so it checks the context and when the context is categories, check the extension it applies to (I have the info in the URL, which is all good).
two lists would be better because the contents are semantically two seperate lists. failing that role=presentation would be a good compromise to solve at least the bad accessibility. As for bootstrap's documentation - remember that their focus is not accessibility or web standards. Acccessibility is often an addendum to any bootstrap documentation if at all.
Brian @brianteeman I put the markup back as it was prior to this PR. I have just added the buttons at the top and hr between the sections. The ul/li was 'breaking' the UI but now I want to make sure we nailed the accessibility of the dropdown, especially when it comes to the hr tag.
You realise that with this change the articles category tour is now emphasised in every component that supports categories?
Fixed.
| Labels |
Added:
Feature
|
||
making it a list was an improvement
making it a list was an improvement
I thought so too... That's why I did it in the first place. I will give it another try.
Side note: the user menu would also benefit from using lists...
| Category | SQL Administration com_admin Postgresql Modules Installation | ⇒ | SQL Administration com_admin Postgresql Modules Repository NPM Change Installation |
OK Brian @brianteeman, I made an attempt with lists.
I have also changed the icon for contextual tours, not sure if it adds much value.
| Labels |
Added:
NPM Resource Changed
|
||
| Category | SQL Administration com_admin Postgresql Modules Installation Repository NPM Change | ⇒ | SQL Administration com_admin Postgresql Modules Installation |
| Labels |
Removed:
NPM Resource Changed
|
||
| Category | SQL Administration com_admin Postgresql Modules Installation | ⇒ | SQL Administration com_admin Postgresql Language & Strings Modules Installation |
| Labels |
Added:
Language Change
|
||
| Labels |
Removed:
Feature
|
||
I have tested this item ✅ successfully on d562290
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41303.
@obuisard It seems that with the last branch update to 5.0-dev you have merged the "5.0.0-2023-08-28.sql" update SQL scripts which had been meanwhile added with PR #41499 to the 5.0-dev branch with your.
This is ok as long as we don't have beta 1 released, because the "5.0.0-2023-08-28.sql" from PR #41499 also hasn't been released yet, so for updating from alpha 4 to beta 1 it is ok to have both things (that PR and yours) in the same script.
But as soon as beta 1 is out without your PR here, and this PR here would go into beta 2, you will have to use a new script with a newer schema version in its name.
For the same reason, when you now update a 5.0-dev to the patched update package or the custom update URL of this PR here, the update SQL would not run because the schema version is already "5.0.0-2023-08-28".
No need to fix that now, but if that happens again in future: The correct fix would be to revert your changes in the "5.0.0-2023-08-28.sql" scripts and then create new ones with a higher schema version in their name which contain only your stuff. The problem could have been avoided by renaming your scripts before doing the branch update.
I thought I let you know that.
@obuisard It seems that with the last branch update to 5.0-dev you have merged the "5.0.0-2023-08-28.sql" update SQL scripts which had been meanwhile added with PR #41499 to the 5.0-dev branch with your.
This is ok as long as we don't have beta 1 released, because the "5.0.0-2023-08-28.sql" from PR #41499 also hasn't been released yet, so for updating from alpha 4 to beta 1 it is ok to have both things (that PR and yours) in the same script.
But as soon as beta 1 is out without your PR here, and this PR here would go into beta 2, you will have to use a new script with a newer schema version in its name.
For the same reason, when you now update a 5.0-dev to the patched update package or the custom update URL of this PR here, the update SQL would not run because the schema version is already "5.0.0-2023-08-28".
No need to fix that now, but if that happens again in future: The correct fix would be to revert your changes in the "5.0.0-2023-08-28.sql" scripts and then create new ones with a higher schema version in their name which contain only your stuff. The problem could have been avoided by renaming your scripts before doing the branch update.
Thank Richard @richard67 for the explanation, very helpful. I am hoping to get this PR into beta 1.
As I understand:
- if this PR goes to beta 1, I can leave it as is,
- if the PR does NOT go into beta 1, I need to change the file names to a later date, without (of course) the extra code from the other PR.
Thank Richard @richard67 for the explanation, very helpful. I am hoping to get this PR into beta 1. As I understand:
* if this PR goes to beta 1, I can leave it as is, * if the PR does NOT go into beta 1, I need to change the file names to a later date, without (of course) the extra code from the other PR.
Correct.
I have tested this item ✅ successfully on 27fb319
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41303.
I have tested this item ✅ successfully on 27fb319
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41303.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41303.
(sorry for the late response)
These lists should be ul not ordered lists. You only use ordered lists when the data is in sequence (reference https://accessibleweb.com/question-answer/what-is-an-unordered-list-vs-an-ordered-list/)
(sorry for the late response)
These lists should be ul not ordered lists. You only use ordered lists when the data is in sequence (reference https://accessibleweb.com/question-answer/what-is-an-unordered-list-vs-an-ordered-list/)
I have set it that way because the tours are actually ordered, following the order of the administrator's tours view. Do you still think I should change this Brian @brianteeman ?
Yes. See the explanation in the link I provided
| Labels |
Added:
?
|
||
Yes. See the explanation in the link I provided
I did, thanks Brian.
sdwjoomla
- | Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-09-02 01:27:52 |
| Closed_By | ⇒ | sdwjoomla |
The markup for this is invalid as you have the
<hr>seperator as a listitem so it is included in the count of list items but there is nothing to anounce.What you are actually creating is multiple lists and should be marked up a such.
This stackoverflow desccribes it perfectly