Updated: Comment #10

Problem/Motivation

The tour module does not respond to changes on the page. Here, there's a popup which looks good:
Tour
Yet, when the toolbar is opened or when the sidebar moves from left to right, the layout breaks:
Tour
Tour

Proposed resolution

Added a method to refresh the position of the tour and trigger the refresh once the viewport offsets change.

Remaining tasks

Commit patch in #7.

User interface changes

- The popup's position should change according to the events that trigger changes in the layout.

API changes

Files: 
CommentFileSizeAuthor
#17 2224541-17-tour-module-respond-changes.patch973 bytesSam152
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,001 pass(es). View
#13 2224541-12-tour-module-respond-changes.patch976 bytesSam152
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,333 pass(es). View
#7 2224541-7-tour-module-respond-changes.patch909 byteser.pushpinderrana
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,222 pass(es). View
#6 2224541-tour-module-respond-changes.patch.txt909 bytesSam152
#4 2224541-tour-module-respond-changes.patch908 bytesSam152
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,194 pass(es). View
modal-bad-2.png503.86 KBsqndr
modal-bad-1.png495.18 KBsqndr
modal-good.png474.86 KBsqndr

Comments

sqndr’s picture

Priority: Normal » Minor
wwwalker’s picture

Issue tags: +#views #tour

The bug occurs when the menu is changed from horizontal to vertical orientation by clicking on the 'vertical orientation' button on the top right of the browser. But the error does not occur when the menu is changed by resizing the browser, i.e. menu does not overlap the tip box and tip points to the right HTML tag. If the menu is changed from vertical to horizontal by clicking 'horizontal orientation' button, the tip box is pointing to the wrong position and is not following tag until the browser is resized when it points to right tag again. It seems that a side effect of tour.js (Backbone) is not updating its tip position until browser is resized.

paulh’s picture

Possibly a solution is to position the Tours from the corner of the page element, not the corner of the body.

Sam152’s picture

Status: Needs work » Needs review
FileSize
908 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,194 pass(es). View

Added a method to refresh the position of the tour and trigger the refresh once the viewport offsets change.

fenstrat’s picture

Status: Needs review » Needs work

Good work @Sam152. The $tour.joyride('is_phone') seems odd, but it is how joyride does it. Tested and this fixes the issue nicely.

Just a minor nitpick:

+++ b/core/modules/tour/js/tour.js
@@ -118,6 +119,19 @@
+      else{

Missing space between else and {

Sam152’s picture

Thanks for the feedback fenstrat. Fix attached.

I think it's pretty annoying that joyride doesn't provide a method that abstracts this tidbit away. I had considered triggering resize.joyride, but it seemed hackier than the minor duplication.

er.pushpinderrana’s picture

Status: Needs work » Needs review
FileSize
909 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,222 pass(es). View

Rerolled patch.

Status: Needs review » Needs work

The last submitted patch, 7: 2224541-7-tour-module-respond-changes.patch, failed testing.

Status: Needs work » Needs review
fenstrat’s picture

Issue summary: View changes
Status: Needs review » Reviewed & tested by the community

This is good to go. There hasn't been any movement on #2027623: De-fork jQuery Joyride and update to latest stable release (and at a glance it looks like joyride 2.1 will not fix this). This patch fixes a real bug we have, so this is RTBC.

alexpott’s picture

Issue tags: -#views #tour +JavaScript

Tagging for a JS maintainer review.

nod_’s picture

Status: Reviewed & tested by the community » Needs work

This patch makes the JS crash with the error Uncaught TypeError: Cannot read property 'width' of undefined some things are not initialized properly in joyride in those conditions. Putting a if ($tour.length) {} helps on most pages but still breaks other pages. The new refreshPosition method needs to check if a tour is open before doing anything.

The event needs to be namespaced, bind it to drupalViewportOffsetChange.joyride please.

Sam152’s picture

FileSize
976 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,333 pass(es). View

Thanks for the review nod_. Binding to ".joyride" causes the fix to only work once, as joyride destroys all of the events when you close it for the first time, causing subsequent reopens to break. Instead I've namespaced to "tour". Also fixed the inactivty by using the 'isActive' flag.

Sam152’s picture

Status: Needs work » Needs review
nod_’s picture

can you go with this.refreshPosition.bind(this) ? don't need underscore to do that it's native.

Sam152’s picture

Saw that pattern a few other places in core. It saves us from having to go var _this = this; function() { _this.refreshPosition.bind(_this) }, so it is a fair bit more concise.

Sam152’s picture

FileSize
973 bytes
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 75,001 pass(es). View

Misunderstood #16, cleared up on IRC. New patch attached.

nod_’s picture

Status: Needs review » Needs work

The position when playing around with the toolbar isn't really perfect. Maybe a timing thing. When you have a tip pointing at an element opening and closing the menu tray makes the tip point to something irrelevant.

Not sure how to fix.

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.