Skip to content
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

Feature/api tour #1602

Merged
merged 49 commits into from
Sep 23, 2016
Merged

Feature/api tour #1602

merged 49 commits into from
Sep 23, 2016

Conversation

brylie
Copy link
Contributor

@brylie brylie commented Sep 19, 2016

Closes #1434
Closes #1445
Closes #1613

Changes

  • add intro.js
  • create basic introduction on API page

@brylie brylie added this to the Sprint 31 milestone Sep 19, 2016
@brylie
Copy link
Contributor Author

brylie commented Sep 22, 2016

@jykae or @marla-singer: please review.

Ping @bajiat

@marla-singer
Copy link
Contributor

@brylie Reviewing

Copy link
Contributor

@marla-singer marla-singer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basic introduction on API page looks nice 👍 Change some remarks and I will merge it. What about the attributes for links, these is at your discretion

{{_ "aboutApinf_homepage" }}
</dt>
<dd>
<a href="https://apinf.org">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just recommendation for user usability: Tag should have an attribute target="_blank" for the outside links. Then users won't leave the site completely

<a href="#api-metadata" data-toggle="tab">
<i class="fa fa-book"></i>
{{_ "viewApiNavigationMenu_metadata" }}
</a>
</li>
<li>
<li id="api-feedback-tab">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add comment in this line too

@@ -11,7 +11,7 @@
background-color: #fafafa;
border-bottom: 1px solid #eee;

.api-name {
.api-header {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change query selector: you used id instead of class. Maybe the api title looks bad because of it
joxi_screenshot_1474554711992

@@ -0,0 +1,496 @@
.introjs-overlay {
position: absolute;
Copy link
Contributor

@marla-singer marla-singer Sep 23, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try importing the introjs CSS directly from the NPM package:

"apiIntro_steps_feedback_intro": "Users of your API can give feedback, including reporting errors or requesting new features.",
"apiIntro_steps_metadata_intro": "Provide organizational, contact and service related information about your API.",
"apiIntro_steps_proxy_intro": "Use this tab to attach your API to a proxy, allowing you to use various API management related features (e.g. analytics, API keys, etc.).",
"apiIntro_steps_settings_intro": "Edit API settings from this tab. You can also delete also API here.",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edit the second sentence: using the word 'also' twice

@brylie
Copy link
Contributor Author

brylie commented Sep 23, 2016

@marla-singer I made the suggested changes. Please review.

@marla-singer
Copy link
Contributor

@brylie I go ahead and correct query selector by myself. All good. Merging

@marla-singer marla-singer merged commit e5df0e2 into develop Sep 23, 2016
@marla-singer marla-singer deleted the feature/api-tour branch September 23, 2016 10:33
@ilarimikkonen ilarimikkonen added this to In Review in apinf/platform Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants