We should add classes to the tour.module tips so that modules can either theme or provide custom js for a specific set.

CommentFileSizeAuthor
#17 tour-1921132-17.patch4 KBnick_schuch
#8 tour-1921132-8.patch4.47 KBnick_schuch
#6 tour-1921132-6.interdiff.txt2.76 KBnick_schuch
#6 tour-1921132-6.patch0 bytesnick_schuch
#2 tour-1921132-2.patch1.7 KBnick_schuch
#1 tour-1921132-1.interdiff.txt3.89 KBnick_schuch
#1 tour-1921132-1.patch19.03 KBnick_schuch

Comments

nick_schuch’s picture

nick_schuch commented
Status: Active » Needs review
StatusFileSize
new tour-1921132-1.patch19.03 KB
new tour-1921132-1.interdiff.txt3.89 KB

Here is the functionality that will make this happen. This patch also contains #1932048 which will be rerolled out after it is committed. The changes can be seen in the interdiff.

nick_schuch’s picture

nick_schuch commented
StatusFileSize
new tour-1921132-2.patch1.7 KB

Here is a rerolled version now that #1932048 is in core.

Status: Needs review » Needs work

The last submitted patch, tour-1921132-2.patch, failed testing.

nick_schuch’s picture

nick_schuch commented
Status: Needs work » Needs review

#2: tour-1921132-2.patch queued for re-testing.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented 
+++ b/core/modules/tour/lib/Drupal/tour/TourRenderController.phpundefined
@@ -26,6 +26,13 @@ public function viewMultiple(array $entities = array(), $view_mode = 'full', $la
+              'tip-module-' . drupal_clean_css_identifier($entity->get('module')),

Can we get this added as a property on the Tour entity type like in the first patch?

nick_schuch’s picture

nick_schuch commented
StatusFileSize
new tour-1921132-6.patch0 bytes
new tour-1921132-6.interdiff.txt2.76 KB

Thanks larowlan! Good catch!

I have also added tests into this one. I think we are all good now.

nick_schuch’s picture

nick_schuch commented
Status: Needs review » Needs work

Hmm empty patch.

nick_schuch’s picture

nick_schuch commented
Status: Needs work » Needs review
StatusFileSize
new tour-1921132-8.patch4.47 KB

Here we go. More than 0 bytes!

fakingfantastic’s picture

fakingfantastic commented

This patch applied cleanly for me. I tested the Tour manually on a Views tour.

Tip wrapper opening HTML before patch (at commit 0fc4db86b608e3436cadc86be2fcd778f8f6e203):
<div class="joyride-tip-guide odd first" data-index="0" style="visibility: visible; display: block; top: 314px; left: 114px;">

After the patch
<div class="joyride-tip-guide tip-module- tip-type-text tip-views-ui-displays-en odd first" data-index="0" style="visibility: visible; display: block; top: 274px; left: 114px;">

I will note that drupal_clean_css_identifier($entity->get('module')) is returning empty in the patch, hence the class of "tip-module-",

fakingfantastic’s picture

fakingfantastic commented
Status: Needs review » Needs work
nick_schuch’s picture

nick_schuch commented

Thanks for the review fakingfantastic!

You will need a reinstall for this to work as the "module" field on the yml needs to be saved into the active store (has been added in this patch).

fakingfantastic’s picture

fakingfantastic commented

Hey Nick,

I just confirmed you are in fact correct. Did an install of D8 with the patch in place, and the resulting views tooltip now looks like:

<div class="joyride-tip-guide tip-module-views-ui tip-type-text tip-views-ui-displays-en odd first" data-index="0" style="visibility: visible; display: block; top: 317px; left: 176px;">

All good to me :)

fakingfantastic’s picture

fakingfantastic commented
Status: Needs work » Reviewed & tested by the community

I think it's ok to switch the status to this, but let me know if I'm wrong.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

+1 RTBC

xjm’s picture

xjm
she/her
Primary language English
commented

#8: tour-1921132-8.patch queued for re-testing.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, tour-1921132-8.patch, failed testing.

nick_schuch’s picture

nick_schuch commented
Status: Needs work » Needs review
StatusFileSize
new tour-1921132-17.patch4 KB

Rerolled. Waiting on it to come back green.

nick_schuch’s picture

nick_schuch commented
Status: Needs review » Reviewed & tested by the community

Good to go back to rtbc.

xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: +Quick fix
xjm’s picture

xjm
she/her
Primary language English
commented

#17: tour-1921132-17.patch queued for re-testing.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed 717241a and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.