- Issue created by @thejimbirch
- 🇩🇪Germany rkoller Nürnberg, Germany
at first i agree not showing a button if there is no tour available is the right thing to do here.
it looks like you are using gin. does "Remove the colors on the tour links, as was the behavior in Drupal core." refer to just gin then? cuz there the gin light grey text color against the blue button background has a color contrast of 1.1:1, with the regular white the button has in claro the necessary contrast of 4.5:1 would be achieved
and one idea for another aspect, in the context of #2961001-23: [META] Improve accessibility of tour module → i've raised the suggestion for adding an hotkey:
And one idea, aside providing the option to the user to start a tour by pressing the tour button it might be also helpful for advanced users and screenreader users in particular to add an hot key for starting a tour. That way if a screenreader user gets to a page and presses the hot key either the tour starts or you could add an announcement, something like that there is no tour for this page available. That way there is no need to get to the "start tour"-button and the entire process is streamlined.
sighted users still dont need any "there is no tour"- button but for screenreader users you could then add an announcement in case the hotkey is pressed and there is no tour available on this page confirming that fact.
- 🇺🇸United States smustgrave
As of right now don’t think I’m ready to rever the change of hiding the tour. Since it solves other problems.
But 100% open to adding a settings form to allow sites to control the text.
Would that be a good compromise?
- 🇺🇸United States thejimbirch Cape Cod, Massachusetts
does "Remove the colors on the tour links, as was the behavior in Drupal core." refer to just gin then?
No. I agree it is not accessible in Gin. I am asking to remove the colors because...
1. Not having a Tour is a large red error color. It is not an error to not have a tour on a page. Most pages will not have tours, so you will have a big red error on most admin pages.
2. No other links in the toolbar have big blue backgrounds. What makes the tour so special that it needs the background colors? The tour is a tool in the toolbar; it should be equal to the other tools.
But 100% open to adding a settings form to allow sites to control the text.
If that were stored in a config like tour.settings.yml so we could change it using recipes, that would be great.
I think that in combination with removing the background colors would be great.
- First commit to issue fork.
- Status changed to Needs review
7 months ago 2:31pm 25 August 2024 Added new config for tour text available & tour not available text , removed the background colour, also added menu items as so that user can access the new config from the UI
Please review, moving NR.
- Status changed to Needs work
7 months ago 4:08pm 25 August 2024 - 🇺🇸United States smustgrave
Looks good! Left some comments
For the link, agree we 100% need it. But lets role it into the Tour.
So on the configuration page it would just be Tour. click into it and there is a tab for "Settings"
- 🇩🇪Germany rkoller Nürnberg, Germany
Two thoughts. Would it make sense to add a local items so you have one item for the list of tours and one local item for the newly introduced settings page? and the other point would it make sense to add an option to reset the two button labels back to the default?
- 🇩🇪Germany rkoller Nürnberg, Germany
and in regards of the microcopy how about the following suggestions:
Configure tour settings -> Configure button label
*the h1 has already set the overall context that the page is about tour and its settings. i would make the title of this field set about, what the field set is actually about. So maybe simply go with "Configure button label"?Tour available text -> Tour available
*i would strike the word text, plus i would rather clarify that the field is about the button label within its description
Tour text of page for which tour available -> Button label for pages with a tour availableTour not available text -> No tour available
*i would strike the word text, plus i would rather clarify that the field is about the button label within its description
Tour text of page for which tour not available -> Button label for pages with no tour available - Status changed to Needs review
7 months ago 5:58pm 25 August 2024 Addressed the feedback on the MR & #12 🐛 "No tour available for this page" message is larger than the tour button and unhelpful Needs review
So on the configuration page it would just be Tour. click into it and there is a tab for "Settings"
I believe we should not add it as local item , similar suggestion #11 🐛 "No tour available for this page" message is larger than the tour button and unhelpful Needs review , any thoughts over it?
- 🇺🇸United States smustgrave
Sorry going to disagree and add as local task links. Having 2 links on the config page doesn’t seem correct.
Don’t know a lot of modules that break it out like that.
- 🇩🇪Germany rkoller Nürnberg, Germany
true. not many do it. the only two examples that i know would be:
admin/structure/views
admin/appearancebut on the other hand also not many modules have two or more similarly named menu items... the navigation module does that, media and admin toolbar. that was the thought behind to keep the associated settings pages contained and easier to process to the user.
Added link as local item attached screenshot as well, apart from it nothing seems to be left.
PLease review, already in NR
-
smustgrave →
committed 885f6f81 on 2.0.x
Issue #3468666 by pooja_sharma, thejimbirch, rkoller: "No tour available...
-
smustgrave →
committed 885f6f81 on 2.0.x
- Status changed to Fixed
7 months ago 1:02pm 26 August 2024 - 🇺🇸United States smustgrave
Was my fault I meant something like
But I took care of it. I was actually targeting this one for the following release but since it got done going to include.
Should we create separate ticket for it to make link menu like above mentioned screenshot.
- 🇺🇸United States smustgrave
Nope already took care of it directly in the commit Thanks!
Automatically closed - issue fixed for 2 weeks with no activity.