Enable tour features #639

ZachTRice · 2017-12-08T20:40:00Z

This is a WIP...

Changes in this pull request:

Enable tour config feature in main.js
Add jquery-ui dialog
Change name of Google analytics import name to be consistent with codebase
Add autoStart: true to initiate joyride v2.2.0
Change feedback.js to export default
Fix stylesheet differences between joyride 2.0 and 2.2
Re-implement page counter.*
Note*: The previous version of joyride was customized directly to add this feature:
https://github.com/nasa-gibs/worldview/search?utf8=%E2%9C%93&q=wv-tour-page-count
Restore Previous buttons

- Enable tour config feature in main.js - Add jquery-ui dialog - Change name of Google analytics import name to be consistent with codebase

- Add `autoStart: true` to initiate joyride v2.2.0 - Change feedback.js to export default

- Remove unsupported data-options - Move depreciated data-option joyride options to html classes - Fix multiple HTML errors in tour.html and format the HTML properly - Create CSS to style the tour windows as they were in previous joyride version* *Note !important classes were used to overwrite !important declarations found in the defauly joyride stylesheets

… into module-loaders-tour

- Restore step page indicator - Fix styles related to modal arrow and button hover state

ZachTRice · 2017-12-21T20:15:45Z

@bking, @localjo I can't seem to get the previous buttons to show up. If you get a chance to break away and look at this feature, here is where I am at:

I see joyride examples with previous buttons here: http://zurb.github.io/joyride/

But then I see references to the functionality not working for others, see:
zurb/joyride#184

And it is nowhere mentioned on the official page:
https://zurb.com/playground/jquery-joyride-feature-tour-plugin

Looking at the version we are running on production, it looks like there was significant modification to the previous button code:
https://github.com/nasa-gibs/worldview/blob/e8c796d012ada1f7e9118c27230391dd58042963/web/ext/tour/joyride-2.0.3-3/joyride.js
(If you go through and search for the comment "//BETH" all the code around it seems to have been modified at some point from the original.

Also a few commits related to previous button in the file blame suggest the same
https://github.com/nasa-gibs/worldview/blame/e8c796d012ada1f7e9118c27230391dd58042963/web/ext/tour/joyride-2.0.3-3/joyride.js

If there is no obvious solution here, then it might be best to test the fix linked above, fork the repo, add that fix and then link to our fork in package.json.

Benjaki2 · 2017-12-21T21:20:24Z

I took a look and agree that a fork is probably necessary.

The working example (http://zurb.github.io/joyride/) isn't using Jquery-joyride so maybe the non-jquery version would work for us if it isn't a lot of work to migrate?

localjo · 2017-12-26T13:54:13Z

web/css/tour.css

@@ -22,6 +22,10 @@
  width: 450px;
 }

+.joyride-close-tip {


We didn't have custom .joyride CSS before, and we're including the Joyride stylesheet in this branch already.

We shouldn't have to add custom CSS to our stylesheets to achieve style parity with production. @ZachTRice @Benjaki2 I might have missed something when I included the dependency stylesheets. Maybe the paths aren't correct, or maybe they're in the wrong order or something. Both of you have added dependency styles to stylesheets in your PRs, but I think maybe there's a problem with how I did the CSS originally. Have either of you looked into that?

The version we were using before was modified to add css directly to the plugin. Here is the css blame: https://github.com/nasa-gibs/worldview/blame/master/web/ext/tour/joyride-2.0.3-3/joyride.css

localjo · 2017-12-26T13:54:57Z

web/pages/tour.html

@@ -119,6 +119,7 @@ <h3>Timeline</h3>
        <li>Change the interval to "Days" and explore the imagery for that month.</li>
      </ul>
    </div>
+    <p class="wv-tour-page-count">(page 1 of 6)</p>


How were these <p> tags being added before?

The plugin was a complete hack before. These were added into the core plugin before. There is no native support for this type of functionality.

localjo · 2018-01-02T18:53:50Z

web/js/feedback.js

@@ -30,4 +29,4 @@ export const feedbackModal = (function () {
  };

  return self;
-})();
+})({});


Why did you move self from being defined inside the function to being passed in as a parameter?

Discussed in slack chat; export constant does not work in this case. We can look at changing this to a named function when we do a review. Util functions are setup in a similar manner as this.

Does a naming the function change how parameters are passed in? Before, you could call this function with no parameters, feedbackModal(), and it would work fine. Now we have to call the function with an empty object for it to work, feedbackModal({}). Why can't we do var self = {} inside the function like it was before?

Good call, I have made this change and tested it.

localjo · 2018-01-02T18:58:18Z

web/js/feedback.js

@@ -7,8 +7,7 @@

 import util from './util/util';

-export const feedbackModal = (function () {
-  var self = {};
+export default (function (self) {


Why go from a named export to an unnamed default export?

Discussed in slack chat; export constant does not work in this case. We can look at changing this to a named function when we do a review. Util functions are setup in a similar manner as this.

- Add fork version of jquery.joyride which is a fork of the npm module jquery.joyride and includes a fix to add previous buttons - Add styling for previous buttons - Add jquery-ui dark theme

- Fix page count position on last step - Fix dialog background from not closing properly - Fix tour completion window appearing when clicking previous on last step - Add css to move nub position to the right on the last tour step

localjo

Looks good to me!

Enable tour features

c30d674

- Enable tour config feature in main.js - Add jquery-ui dialog - Change name of Google analytics import name to be consistent with codebase

ghost assigned ZachTRice Dec 8, 2017

ghost added the under development label Dec 8, 2017

ZachTRice changed the base branch from master to module-loaders December 8, 2017 20:40

Fix joyride not initating on click and errors related to feedback.js

9a7f1c3

- Add `autoStart: true` to initiate joyride v2.2.0 - Change feedback.js to export default

ZachTRice force-pushed the module-loaders-tour branch from 0962a9c to 9a7f1c3 Compare December 8, 2017 20:46

ZachTRice added 4 commits December 13, 2017 12:36

Merge branch 'module-loaders' of https://github.com/nasa-gibs/worldview…

54fa7d2

… into module-loaders-tour

Merge branch 'module-loaders' of https://github.com/nasa-gibs/worldview…

ad3509a

… into module-loaders-tour

Restore tour pagination and fix styles

de3b251

- Restore step page indicator - Fix styles related to modal arrow and button hover state

Benjaki2 mentioned this pull request Dec 22, 2017

Modularize notification feature #644

Merged

2 tasks

localjo suggested changes Dec 26, 2017

View reviewed changes

Benjaki2 mentioned this pull request Jan 2, 2018

Module loaders #503

Merged

58 tasks

localjo reviewed Jan 2, 2018

View reviewed changes

ZachTRice added 3 commits January 2, 2018 14:44

Restore previous buttons to jquery joyride

de3c47f

- Add fork version of jquery.joyride which is a fork of the npm module jquery.joyride and includes a fix to add previous buttons - Add styling for previous buttons - Add jquery-ui dark theme

Move parameters inside the feedback function and assign to self

cc0fc12

localjo approved these changes Jan 2, 2018

View reviewed changes

localjo merged commit e5dcbdf into module-loaders Jan 3, 2018

ghost removed the under development label Jan 3, 2018

localjo deleted the module-loaders-tour branch January 3, 2018 17:51

localjo added this to the Module Loaders Release (v2.0) milestone Jan 12, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable tour features #639

Enable tour features #639

ZachTRice commented Dec 8, 2017 •

edited

Loading

ZachTRice commented Dec 21, 2017

Benjaki2 commented Dec 21, 2017

localjo Dec 26, 2017

ZachTRice Jan 2, 2018 •

edited

Loading

localjo Dec 26, 2017

ZachTRice Jan 2, 2018

localjo Jan 2, 2018

ZachTRice Jan 2, 2018

localjo Jan 2, 2018

ZachTRice Jan 2, 2018

localjo Jan 2, 2018

ZachTRice Jan 2, 2018

localjo left a comment

Enable tour features #639

Enable tour features #639

Conversation

ZachTRice commented Dec 8, 2017 • edited Loading

ZachTRice commented Dec 21, 2017

Benjaki2 commented Dec 21, 2017

Choose a reason for hiding this comment

ZachTRice Jan 2, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

localjo left a comment

Choose a reason for hiding this comment

ZachTRice commented Dec 8, 2017 •

edited

Loading

ZachTRice Jan 2, 2018 •

edited

Loading