-
Notifications
You must be signed in to change notification settings - Fork 20
[WIP] Add tutorials describing basic features of the product #107
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
f0c3cb4 to
602afa2
Compare
2961357 to
bd05a0a
Compare
04d42a1 to
4720f73
Compare
OlegLazaryev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed.
Let's discuss.
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
|
|
||
| }); | ||
|
|
||
| subscribeHighlightingElement(step1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is duplicated several times. What in this case makes it hardly maintainable. I have some ideas how to generalize it. Let's discuss in-person.
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
| } | ||
| }); | ||
|
|
||
| var step1 = tour.addStep('services-tab', { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an idea of how to generalize the creation of steps. I think we can introduce a kind of a factory method to do that. Let's discuss in-person.
OlegLazaryev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@volodymyrtymtsiv reviewed.
Let's discuss.
app/services/tutorial.service.js
Outdated
| var selector = getSelector(step); | ||
|
|
||
| step.on('show', function () { | ||
| angular.element(selector).addClass('highlighted-element'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use jQuery selector instead of Angular.
app/services/tutorial.service.js
Outdated
| angular.element(selector).addClass('highlighted-element'); | ||
| }); | ||
| step.on('before-hide', function () { | ||
| angular.element(selector).removeClass('highlighted-element') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use jQuery selector instead of Angular.
app/admin_components/adf-tutorial/service-tutorial.directive.js
Outdated
Show resolved
Hide resolved
OlegLazaryev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@volodymyrtymtsiv
Left you some more comments.
I think it is going to be the last round of review.
We will improve the remaining technical debt in the next related PRs.
Let me now when finished.
45f9c4e to
0addeef
Compare
efb4071 to
84ef713
Compare
Refactor
…-scripting Feature/df tutorial scripting
|
@wjgilmore please review. |
|
Thank you will review this week. |
wjgilmore
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@volodymyrpoli do not merge this change. We now do this in Intercom.

Uh oh!
There was an error while loading. Please reload this page.