Skip to content

[WIP] Add tutorials describing basic features of the product#107

Open
vtymtsiv wants to merge 28 commits intomasterfrom
feature/DF-shepherd
Open

[WIP] Add tutorials describing basic features of the product#107
vtymtsiv wants to merge 28 commits intomasterfrom
feature/DF-shepherd

Conversation

@vtymtsiv
Copy link
Copy Markdown

@vtymtsiv vtymtsiv commented Sep 18, 2018

  • Add Service creation tutorial
  • Add Scripting tutorial

Copy link
Copy Markdown
Contributor

@OlegLazaryev OlegLazaryev left a 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.

Comment thread app/admin_components/adf-services/dreamfactory-services.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated

});

subscribeHighlightingElement(step1);
Copy link
Copy Markdown
Contributor

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.

Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
}
});

var step1 = tour.addStep('services-tab', {
Copy link
Copy Markdown
Contributor

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.

Copy link
Copy Markdown
Contributor

@OlegLazaryev OlegLazaryev left a 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.

Comment thread app/admin_components/adf-services/dreamfactory-services.js
Comment thread app/services/tutorial.service.js Outdated
Comment thread app/services/tutorial.service.js Outdated
var selector = getSelector(step);

step.on('show', function () {
angular.element(selector).addClass('highlighted-element');
Copy link
Copy Markdown
Contributor

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.

Comment thread app/services/tutorial.service.js Outdated
angular.element(selector).addClass('highlighted-element');
});
step.on('before-hide', function () {
angular.element(selector).removeClass('highlighted-element')
Copy link
Copy Markdown
Contributor

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.

Comment thread app/admin_components/adf-tutorial/service-tutorial.directive.js Outdated
Comment thread app/scripts/tour_scenarios/create_service.scenario.js Outdated
Comment thread app/scripts/tour_scenarios/create_service.scenario.js Outdated
Comment thread app/scripts/tour_scenarios/create_service.scenario.js Outdated
Comment thread app/scripts/tour_scenarios/create_service.scenario.js Outdated
Comment thread app/scripts/tour_scenarios/create_service.scenario.js Outdated
@OlegLazaryev
Copy link
Copy Markdown
Contributor

A general notice which is looking like a bug:

8ff3eef6_2018 25 09

Copy link
Copy Markdown
Contributor

@OlegLazaryev OlegLazaryev left a 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.

Comment thread app/admin_components/adf-services/dreamfactory-services.js
Comment thread app/admin_components/adf-tutorial/tutorial.controller.js
Comment thread app/scripts/app.js Outdated
Comment thread app/scripts/tour_scenarios/tour_builder.js Outdated
Comment thread app/scripts/tour_scenarios/tour_builder.js Outdated
Comment thread app/admin_components/adf-tutorial/Readme.md Outdated
Comment thread app/admin_components/adf-tutorial/Readme.md Outdated
@OlegLazaryev
Copy link
Copy Markdown
Contributor

@wjgilmore please review.

@wjgilmore
Copy link
Copy Markdown
Contributor

Thank you will review this week.

@vtymtsiv vtymtsiv removed their assignment Feb 10, 2020
@yaroslavmo yaroslavmo closed this May 1, 2020
@yaroslavmo yaroslavmo reopened this May 18, 2020
Copy link
Copy Markdown
Contributor

@wjgilmore wjgilmore left a 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.

Base automatically changed from develop to master June 16, 2020 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants